[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of c
From: |
Jonathan Cameron |
Subject: |
Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers |
Date: |
Wed, 18 Oct 2023 17:22:39 +0100 |
On Wed, 18 Oct 2023 10:40:45 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Oct 12, 2023 at 03:05:12PM +0100, Jonathan Cameron wrote:
> > Establishing that only register accesses of size 4 and 8 can occur
> > using these functions requires looking at their callers. Make it
> > easier to see that by using switch statements.
> > Assertions are used to enforce that the register storage is of the
> > matching size, allowing fixed values to be used for divisors of
> > the array indices.
> >
> > Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Reviewed-by: Fan Ni <fan.ni@samsung.com>
>
> Fails with clang:
>
> ../hw/cxl/cxl-component-utils.c:130:9: error: expected expression
> QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_regs_write_mask) != 4);
> ^
> ../include/qemu/compiler.h:76:30: note: expanded from macro
> 'QEMU_BUILD_BUG_ON'
> #define QEMU_BUILD_BUG_ON(x) QEMU_BUILD_BUG_MSG(x, "not expecting: " #x)
> ^
> ../include/qemu/compiler.h:74:36: note: expanded from macro
> 'QEMU_BUILD_BUG_MSG'
> #define QEMU_BUILD_BUG_MSG(x, msg) _Static_assert(!(x), msg)
> ^
> 1 error generated.
>
> See e.g.
>
> https://gitlab.com/mstredhat/qemu/-/jobs/5320879531
Thanks. Will see if I can figure out what is going wrong.
This code seems cursed. I need to figure out a way to run the CI again.
Ran into credit problems I couldn't solve last time I tried.
In meantime I can replicate this locally but not immediately obvious to me
why it is failing.
Jonathan
>
>
>
> > ---
> > v4: Use QEMU_BUILD_BUG_ON() instead of static_assert with missing
> > error message.
> >
> > hw/cxl/cxl-component-utils.c | 65 +++++++++++++++++++++++-------------
> > 1 file changed, 42 insertions(+), 23 deletions(-)
> >
> > diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> > index f3bbf0fd13..d1997a52e6 100644
> > --- a/hw/cxl/cxl-component-utils.c
> > +++ b/hw/cxl/cxl-component-utils.c
> > @@ -67,16 +67,24 @@ static uint64_t cxl_cache_mem_read_reg(void *opaque,
> > hwaddr offset,
> > CXLComponentState *cxl_cstate = opaque;
> > ComponentRegisters *cregs = &cxl_cstate->crb;
> >
> > - if (size == 8) {
> > + switch (size) {
> > + case 4:
> > + if (cregs->special_ops && cregs->special_ops->read) {
> > + return cregs->special_ops->read(cxl_cstate, offset, 4);
> > + } else {
> > + QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
> > + return cregs->cache_mem_registers[offset / 4];
> > + }
> > + case 8:
> > qemu_log_mask(LOG_UNIMP,
> > "CXL 8 byte cache mem registers not implemented\n");
> > return 0;
> > - }
> > -
> > - if (cregs->special_ops && cregs->special_ops->read) {
> > - return cregs->special_ops->read(cxl_cstate, offset, size);
> > - } else {
> > - return cregs->cache_mem_registers[offset /
> > sizeof(*cregs->cache_mem_registers)];
> > + default:
> > + /*
> > + * In line with specifiction limitaions on access sizes, this
> > + * routine is not called with other sizes.
> > + */
> > + g_assert_not_reached();
> > }
> > }
> >
> > @@ -117,25 +125,36 @@ static void cxl_cache_mem_write_reg(void *opaque,
> > hwaddr offset, uint64_t value,
> > ComponentRegisters *cregs = &cxl_cstate->crb;
> > uint32_t mask;
> >
> > - if (size == 8) {
> > + switch (size) {
> > + case 4:
> > + QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_regs_write_mask) != 4);
> > + QEMU_BUILD_BUG_ON(sizeof(*cregs->cache_mem_registers) != 4);
> > + mask = cregs->cache_mem_regs_write_mask[offset / 4];
> > + value &= mask;
> > + /* RO bits should remain constant. Done by reading existing value
> > */
> > + value |= ~mask & cregs->cache_mem_registers[offset / 4];
> > + if (cregs->special_ops && cregs->special_ops->write) {
> > + cregs->special_ops->write(cxl_cstate, offset, value, size);
> > + return;
> > + }
> > +
> > + if (offset >= A_CXL_HDM_DECODER_CAPABILITY &&
> > + offset <= A_CXL_HDM_DECODER3_TARGET_LIST_HI) {
> > + dumb_hdm_handler(cxl_cstate, offset, value);
> > + } else {
> > + cregs->cache_mem_registers[offset / 4] = value;
> > + }
> > + return;
> > + case 8:
> > qemu_log_mask(LOG_UNIMP,
> > "CXL 8 byte cache mem registers not implemented\n");
> > return;
> > - }
> > - mask = cregs->cache_mem_regs_write_mask[offset /
> > sizeof(*cregs->cache_mem_regs_write_mask)];
> > - value &= mask;
> > - /* RO bits should remain constant. Done by reading existing value */
> > - value |= ~mask & cregs->cache_mem_registers[offset /
> > sizeof(*cregs->cache_mem_registers)];
> > - if (cregs->special_ops && cregs->special_ops->write) {
> > - cregs->special_ops->write(cxl_cstate, offset, value, size);
> > - return;
> > - }
> > -
> > - if (offset >= A_CXL_HDM_DECODER_CAPABILITY &&
> > - offset <= A_CXL_HDM_DECODER3_TARGET_LIST_HI) {
> > - dumb_hdm_handler(cxl_cstate, offset, value);
> > - } else {
> > - cregs->cache_mem_registers[offset /
> > sizeof(*cregs->cache_mem_registers)] = value;
> > + default:
> > + /*
> > + * In line with specifiction limitaions on access sizes, this
> > + * routine is not called with other sizes.
> > + */
> > + g_assert_not_reached();
> > }
> > }
> >
> > --
> > 2.39.2
>
>
- [PATCH v4 0/4] hw/cxl: Line length reduction and related, Jonathan Cameron, 2023/10/12
- [PATCH v4 1/4] hw/cxl: Use a switch to explicitly check size in caps_reg_read(), Jonathan Cameron, 2023/10/12
- [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers, Jonathan Cameron, 2023/10/12
- Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers, Michael S. Tsirkin, 2023/10/18
- Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers,
Jonathan Cameron <=
- Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers, Jonathan Cameron, 2023/10/18
- Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers, Jonathan Cameron, 2023/10/19
- Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers, Michael S. Tsirkin, 2023/10/19
- Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers, Jonathan Cameron, 2023/10/19
- Re: [PATCH v4 2/4] hw/cxl: Use switch statements for read and write of cachemem registers, Jonathan Cameron, 2023/10/19
[PATCH v4 3/4] hw/cxl: CXLDVSECPortExtensions renamed to CXLDVSECPortExt, Jonathan Cameron, 2023/10/12
[PATCH v4 4/4] hw/cxl: Line length reductions, Jonathan Cameron, 2023/10/12