Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro, Zhuohao Lee.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57070 )
Change subject: mb/google/volteer: write tcss config to coreboot tables
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/volteer/variants/volteer2/variant.c:
https://review.coreboot.org/c/coreboot/+/57070/comment/f0a1af08_efaf5d9a
PS1, Line 47: /* Initialize port 0 information */
: info[0].usb2_port_number = 9;
: info[0].usb3_port_number = 1;
: info[0].sbu_orientation = TYPEC_ORIENTATION_FOLLOW_CC;
: info[0].hsl_orientation = TYPEC_ORIENTATION_FOLLOW_CC;
:
: /* Initialize port 1 information */
: info[1].usb2_port_number = 4;
: info[1].usb3_port_number = 2;
: info[1].sbu_orientation = TYPEC_ORIENTATION_FOLLOW_CC;
: info[1].hsl_orientation = TYPEC_ORIENTATION_FOLLOW_CC;
:
This information is already present in devicetree.cb: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/thiā¦
I don't think we should repeat it again in .c files. Ideally, I think this table should be filled/generated by drivers/intel/pmc_mux?
--
To view, visit https://review.coreboot.org/c/coreboot/+/57070
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5c28d8487f7abb92a19a3140d56a8bd437306a5d
Gerrit-Change-Number: 57070
Gerrit-PatchSet: 1
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Comment-Date: Sat, 21 Aug 2021 05:18:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Tim Wawrzynczak, Nick Vaccaro, Zhuohao Lee.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57069 )
Change subject: coreboot tables: Add tcss information to coreboot table
......................................................................
Patch Set 1:
(14 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57069/comment/86c20acb_e0dc312d
PS1, Line 7: tcss
Rather than saying tcss, probably we should just call this type-c port information. tcss is very Intel-specific and I think the type-c port information structure can be applicable to more than just Intel and would be good to define it that way for reuse on other platforms as well (if required).
File payloads/libpayload/include/coreboot_tables.h:
https://review.coreboot.org/c/coreboot/+/57069/comment/4d65c3bc_dd23d9dd
PS1, Line 85: CB_TAG_TCSS_INFO
`CB_TAG_TYPE_C_INFO`
https://review.coreboot.org/c/coreboot/+/57069/comment/6dd32069_64ddc971
PS1, Line 146: #if CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS)
This can be dropped. We don't really need to add a guard here.
https://review.coreboot.org/c/coreboot/+/57069/comment/3712fc29_430f9457
PS1, Line 152: struct tcss_config_info
This needs to be defined in this file. Also, I think this can be organized just like other variable-length entries are organized e.g. `struct mac_address`:
```
struct type_c_port_info {
...
};
struct cb_type_c_info {
u32 tag;
u32 size;
u32 port_count;
struct type_c_port_info ports[0];
};
```
File payloads/libpayload/include/sysinfo.h:
https://review.coreboot.org/c/coreboot/+/57069/comment/1e92ec9e_7f851892
PS1, Line 152: #if CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS)
This can be dropped.
https://review.coreboot.org/c/coreboot/+/57069/comment/a74848ce_935da29b
PS1, Line 155: sbu_orientation
What are the options for the orientation?
https://review.coreboot.org/c/coreboot/+/57069/comment/ced46a64_8fed2ad5
PS1, Line 156: hsl
I think hsl stands for high speed lanes? It would be good to add a comment. Or maybe just say data_orientation to indicate data lines?
https://review.coreboot.org/c/coreboot/+/57069/comment/44bf7ab0_5e4990bf
PS1, Line 159: MAX_TYPE_C_PORTS
This can be defined at the top of this file along with other `SYSINFO_MAX_*` macros
File payloads/libpayload/libc/coreboot.c:
https://review.coreboot.org/c/coreboot/+/57069/comment/3d484036_f8c0ce3c
PS1, Line 249: #if CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS)
This can be dropped.
File src/commonlib/include/commonlib/coreboot_tables.h:
https://review.coreboot.org/c/coreboot/+/57069/comment/034626bb_495a7e65
PS1, Line 6: #include <intelblocks/tcss.h>
This is a common file. Intel-specific header file cannot be included here.
https://review.coreboot.org/c/coreboot/+/57069/comment/264066b8_33ce4155
PS1, Line 425:
: #if CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS)
: /*
: * USB TCSS Configuration Information
: * This record contains board-specific TCSS configuration information.
: * There will be one record per type-C port.
: */
: struct tcss_config_info {
: uint8_t usb2_port_number;
: uint8_t usb3_port_number;
: uint8_t sbu_orientation;
: uint8_t hsl_orientation;
: };
:
: struct lb_tcss_info {
: uint32_t tag;
: uint32_t size;
: uint32_t port_count;
: struct tcss_config_info tcss_info[MAX_TYPE_C_PORTS];
: };
: #endif
Same comments as the payload file.
File src/lib/coreboot_table.c:
https://review.coreboot.org/c/coreboot/+/57069/comment/59390c32_48a6262a
PS1, Line 36: #if CONFIG(SOC_INTEL_COMMON_BLOCK_TCSS)
Same comment as the other file. No Intel-specific configs in common code.
https://review.coreboot.org/c/coreboot/+/57069/comment/8b69887d_e1a4cf57
PS1, Line 242: variant_get_tcss_sysinfo
variant_* shouldn't really be used in common code. It is a mainboard-specific concept. Not all mainboards have variants.
https://review.coreboot.org/c/coreboot/+/57069/comment/12470485_da8d8b6b
PS1, Line 248: lb_add_tcss_info
So, I think this can be organized slightly differently. You can add a weak function:
```
__weak void lb_add_tcss_info(struct lb_header *header) {}
```
and let the actual function be implemented by SoC or EC code.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57069
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ice732be2fa634dbf31ec620552b383c4a5b41451
Gerrit-Change-Number: 57069
Gerrit-PatchSet: 1
Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com>
Gerrit-Comment-Date: Sat, 21 Aug 2021 05:09:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57068 )
Change subject: soc/amd/common/fsp/fsp_validate: check FSP-M binary size
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57068/comment/79f66a55_ca52fe8c
PS2, Line 13: SoCs can implement the soc_validate_fsp_version function
: that ends up being called by the FSP driver in romstage to check the FSP
: version, but since the image size is also present in the header we can
: also implement and use this function to check if it fits into the
: reserved region.
I think it would be better to have a compile-time check to ensure that the build fails rather than having a bad firmware image. It should be possible to do this within soc/amd/common without impacting any other platforms.
```
# git grep -l FSP_M_SIZE
src/soc/amd/cezanne/Kconfig
src/soc/amd/cezanne/root_complex.c
src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld
src/soc/amd/picasso/Kconfig
src/soc/amd/picasso/root_complex.c
```
Probably add a check in src/soc/amd/common/block/cpu/noncar/Makefile.inc which uses file-size macro which is already defined in top-level Makefile.inc to calculate size of FSP_M_FILE and compare that to FSP_M_SIZE.
--
To view, visit https://review.coreboot.org/c/coreboot/+/57068
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9b74a2d03993ba50b166eb6e87d4e57b93afc069
Gerrit-Change-Number: 57068
Gerrit-PatchSet: 2
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Sat, 21 Aug 2021 03:34:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Jakub Czapiga, Jan Dabros.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56813 )
Change subject: tests: Add lib/cbfs-lookup-test test case
......................................................................
Patch Set 7:
(28 comments)
File tests/lib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/56813/comment/d55d4c3a_0e98f0e8
PS7, Line 222: cbmem_top
Unused now?
https://review.coreboot.org/c/coreboot/+/56813/comment/cc1e3066_99b74846
PS7, Line 226: CONFIG_CBFS_VERIFICATION=0
This is the default anyway (and should remain so for the foreseeable future).
File tests/lib/cbfs-lookup-test.c:
https://review.coreboot.org/c/coreboot/+/56813/comment/7e23ce93_20e7169a
PS4, Line 115: BE32(CBFS_FILE_ATTR_TAG_UNUSED),
> So what about CBFS_FILE_ATTR_TAG_UNUSED and CBFS_FILE_ATTR_TAG_UNUSED2? They are here only for cbfst [ā¦]
Well, as the name says they're unused. They're just defined in the enum to block that value off so that nobody accidentally defines a new attribute with value 0x00 or 0xff (which for various reasons could be a bad idea). They are never valid in a real CBFS image
https://review.coreboot.org/c/coreboot/+/56813/comment/e981031c_6a09756c
PS4, Line 139: },
> Now all tests are executed for aligned and unaligned buffer (in terms of address alignment, not CBFS_ALIGNMENT)
Right, but here I actually meant alignment in relation to the start of the rdev (cbfs_walk() should skip headers not on an alignment boundary, test should verify that behavior).
> I think, that these cases are covered by those:
Well yes, it's kinda the same thing, but I think it might be worth having both (one where it exceeds the available size by a little bit, and one where it uses a number very close to UINT32_MAX). I could imagine failure cases where the latter causes some sort of overflow or is incorrectly interpreted as a signed value somewhere.
https://review.coreboot.org/c/coreboot/+/56813/comment/823fc2e4_df1d32a4
PS4, Line 148: __aligned(CBFS_ALIGNMENT);
> Done. However, I see that CBFS "aligns" files by CBFS_ALIGNMENT, but does not use ALIGN_UP/DOWN. [ā¦]
Sorry, not quite sure what you mean. Are you talking about where the image is parsed, or where it is generated? The parser uses ALIGN_UP (read_next_header() in src/commonlib/bsd/cbfs_private.c). cbfstool also respects the alignment, although it's a bit more complicated to follow why... cbfstool never really "creates" a CBFS file header in empty space, because all empty space is already covered by CBFS_TYPE_DELETED file entries. So all cbfstool ever does is split CBFS_TYPE_DELETED files into multiple smaller entries, and aligning those is handled by cbfs_add_entry_at() and cbfs_find_next_entry().
https://review.coreboot.org/c/coreboot/+/56813/comment/8211dede_8213b900
PS4, Line 240: cbd.mcache_size = TEST_MCACHE_SIZE;
> Each test calls cbfs_init_boot_device() after constructing CBFS. [ā¦]
Okay, for this test sounds fine. But eventually we should also have a separate mcache test that tests all the edge cases for mcache_build specifically.
File tests/lib/cbfs-lookup-test.c:
https://review.coreboot.org/c/coreboot/+/56813/comment/4519ca27_83ad32ed
PS7, Line 35: BE64
nit: as long as it's test data it doesn't really matter, but be aware that while CBFS header fields and attributes are always big-endian, the file *contents* have no such convention and are usually just in host byte order of the stage that accesses them. (And cbfstool add-integer actually seems to explicitly encode numbers as little-endian for some reason, as far as I can tell.)
https://review.coreboot.org/c/coreboot/+/56813/comment/73d0f834_f3e4e92c
PS7, Line 80: break
Should this not also assert()? (Could pull this assert() out below the if-else block, I think...)
https://review.coreboot.org/c/coreboot/+/56813/comment/4384c2ad_4efdea25
PS7, Line 90: if (offset % CBFS_ALIGNMENT != 0)
nit: technically don't need to check first, ALIGN_UP() will do the right thing either way.
https://review.coreboot.org/c/coreboot/+/56813/comment/a76dc96e_47a16bb7
PS7, Line 120: #if CONFIG(NO_CBFS_MCACHE)
nit: Should be no need to only compile one of the two? Should be okay if the other one just doesn't get called. In fact, defining it (with a call to mock_type()) when we don't expect it to get called serves to verify that it actually doesn't get called.
https://review.coreboot.org/c/coreboot/+/56813/comment/68735e0d_0d41de7a
PS7, Line 154: {
nit: might as well throw in an assert_ptr_equal(mp, _cbfs_cache) to both of these?
https://review.coreboot.org/c/coreboot/+/56813/comment/dd9fd9a5_71165b19
PS7, Line 185: size_t sz;
nit: I'd maybe call these cbfs_buf and cbfs_size or img_buf and img_size, to make it more obvious in the code below what buffer exactly this represents (because things are getting pretty complicated down there).
https://review.coreboot.org/c/coreboot/+/56813/comment/78eac6d1_8d5b1d14
PS7, Line 198: s->sz = sizeof(aligned_cbfs_buffer);
Maybe zero out s->ex just in case?
https://review.coreboot.org/c/coreboot/+/56813/comment/ea085dc0_c70d3121
PS7, Line 248: #define expect_lookup_result(res) \
nit: can't this just be a function?
https://review.coreboot.org/c/coreboot/+/56813/comment/00b9885a_7e8f476f
PS7, Line 439: &s->buf[5]
So, this is okay but now you have an image that doesn't even start with a CBFS header, which could also be a possible cause of failure. I think it would be good to test that while the unaligned files in this cannot be found, and aligned header somewhere in the middle of this would still be valid. (I think it'll be easiest to just memcpy() the CBFS entries together by hand for this, the create_cbfs() function can't cover all those special cases.)
https://review.coreboot.org/c/coreboot/+/56813/comment/ba05e5f7_1ca408e0
PS7, Line 472: assert_int_equal(0, size_out);
This isn't true... in fact, I believe the current implementation will write the size claimed in the header to size_out, even if the mapping fails afterwards. But that should not be part of the API, the caller should just assume size_out is undefined if the mapping fails, so tests shouldn't check it in that case.
https://review.coreboot.org/c/coreboot/+/56813/comment/b5df61dc_1e5cc51e
PS7, Line 475: static void test_cbfs_fail_beyond_rdev(void **state)
This whole construction is so complicated that I think you should build a success case for it as well, just to make sure that the intentional failures you're seeing really fail for the reasons you think. Otherwise it's easily possible that there's something wrong in the setup that just makes everything you do with this test fail. (Would need to store expected success or failure in s->ex as well, then.)
https://review.coreboot.org/c/coreboot/+/56813/comment/9f79576d_09def42b
PS7, Line 491: full_file_size
I believe you want sizeof(test_file_to_go_beyond) here, because full_file_size may be much larger (and you'd end up copying invalid memory).
https://review.coreboot.org/c/coreboot/+/56813/comment/563882ce_90cd42da
PS7, Line 492: test_file_1
Below you're looking for TEST_DATA_2_FILENAME, so I think you want &test_file_2?
https://review.coreboot.org/c/coreboot/+/56813/comment/5e0ba656_5752008a
PS7, Line 526: mapping = cbfs_map("abcdefghijklmnop", &size_out);
This is a good test to have, but it could still fail to check a problem where the code just strcmp()s over the end of the filename field and just doesn't match this file because there are more "letters" (misinterpreted bytes from the attributes or file data) that are not in the name you're looking for. A second, complementary test for that case could be done by copying test_file_1 into the image, then changing the (struct cbfs_file).offset field to be32(offsetof(filename) + strlen(TEST_DATA_1_FILENAME)) (which should come out one short of the null terminator and thus make it fail)).
https://review.coreboot.org/c/coreboot/+/56813/comment/84b3f4bd_6e78c559
PS7, Line 537: const struct cbfs_test_file test_file = {
nit: maybe easier to copy in one of your pre-existing files and then just change one field?
https://review.coreboot.org/c/coreboot/+/56813/comment/0d3337a6_1e9ef159
PS7, Line 545: .offset = cpu_to_be32(sizeof(struct cbfs_file) + FILENAME_SIZE),
Now that I think of it it would also be good to have another test where the attribute is cut off in the middle, i.e. after the first or second uint32_t. This could be achieved by making offset just 4 or 8 larger than attributes_offset.
https://review.coreboot.org/c/coreboot/+/56813/comment/adcf0ea3_7ce22a6f
PS7, Line 617: .attributes_offset = cpu_to_be32(
This should have no attributes so it doesn't overlap so much with the other test above.
https://review.coreboot.org/c/coreboot/+/56813/comment/2dd4167b_7ae7243d
PS7, Line 730: static void test_cbfs_length_uint32max(void **state)
Would be nice to have this for attributes_offset as well.
https://review.coreboot.org/c/coreboot/+/56813/comment/46a13460_361a93e7
PS7, Line 757: setup_test_cbfs_aligned
I haven't checked exactly how this works, but can you use the group_setup/group_teardown stuff from cmocka_run_group_tests() to cover this? That would be a lot cleaner than having to duplicate all the boilerplate all over the place.
https://review.coreboot.org/c/coreboot/+/56813/comment/30b437bc_3394f0d3
PS7, Line 851: attrs_and_data
Rather than only testing just the edge, it would be nice to also test a little in the middle (e.g. either 4 or 8 bytes further in, to see if having only the attribute tag or the tag and size throw it off somehow.).
https://review.coreboot.org/c/coreboot/+/56813/comment/e3f97ccc_884ff402
PS7, Line 852: CBFS_TYPE_NULL
You say that but your code doesn't do it.
https://review.coreboot.org/c/coreboot/+/56813/comment/356b16dd_293ef5f8
PS7, Line 866: filename
Same here, testing when the cutoff is a few bytes further in (but before the null terminator) would also be useful.
--
To view, visit https://review.coreboot.org/c/coreboot/+/56813
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2ebebba1468c19661741de8a8456605b1c5f56b6
Gerrit-Change-Number: 56813
Gerrit-PatchSet: 7
Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com>
Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com>
Gerrit-Comment-Date: Sat, 21 Aug 2021 00:08:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jakub Czapiga <jacz(a)semihalf.com>
Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, Tim Wawrzynczak, Paul Menzel, Raju Rajakumar, Bernardo Perez Priego.
Selma Bensaid has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56247 )
Change subject: mb/intel/adlrvp_m: Enable ELAN touchscreen
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56247/comment/923a9279_da78c34b
PS4, Line 8:
> Please mention the touch panel model in the commit message.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/56247
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I37c0831485135fda3284dda6b61f4825b7fc51a3
Gerrit-Change-Number: 56247
Gerrit-PatchSet: 5
Gerrit-Owner: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Raju Rajakumar <raju.rajakumar(a)intel.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.corp-partner.google.com>
Gerrit-Attention: Raju Rajakumar <raju.rajakumar(a)intel.com>
Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-Comment-Date: Fri, 20 Aug 2021 23:19:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment