Re: [PATCH qemu 07/11] hw/cxl: mailbox-utils: 0x5602 - FMAPI Set DC Region Config

[Jonathan Cameron via]

On Mon, 14 Jul 2025 05:32:19 -0400
"Michael S. Tsirkin" <m...@redhat.com> wrote:

> On Wed, Jul 02, 2025 at 05:02:13PM +0100, Jonathan Cameron wrote:
> > From: Anisa Su <anisa...@samsung.com>
> > 
> > FM DCD Management command 0x5602 implemented per CXL r3.2 Spec Section 
> > 7.6.7.6.3
> > 
> > Reviewed-by: Fan Ni <fan...@samsung.com>
> > Signed-off-by: Anisa Su <anisa...@samsung.com>
> > Signed-off-by: Jonathan Cameron <jonathan.came...@huawei.com>


> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index bf1710b251..1fc453f70d 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c

> > +/* CXL r3.2 section 7.6.7.6.3: Set Host DC Region Configuration (Opcode 
> > 5602) */
> > +static CXLRetCode cmd_fm_set_dc_region_config(const struct cxl_cmd *cmd,
> > +                                              uint8_t *payload_in,
> > +                                              size_t len_in,
> > +                                              uint8_t *payload_out,
> > +                                              size_t *len_out,
> > +                                              CXLCCI *cci)
> > +{
> > +    struct {
> > +        uint8_t reg_id;
> > +        uint8_t rsvd[3];
> > +        uint64_t block_sz;
> > +        uint8_t flags;
> > +        uint8_t rsvd2[3];
> > +    } QEMU_PACKED *in = (void *)payload_in;
> > +    CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
> > +    CXLEventDynamicCapacity dcEvent = {};
> > +    CXLDCRegion *region = &ct3d->dc.regions[in->reg_id];
> > +
> > +    /*
> > +     * CXL r3.2 7.6.7.6.3: Set DC Region Configuration
> > +     * This command shall fail with Unsupported when the Sanitize on 
> > Release
> > +     * field does not match the region’s configuration... and the device
> > +     * does not support reconfiguration of the Sanitize on Release setting.
> > +     *
> > +     * Currently not reconfigurable, so always fail if sanitize bit (bit 0)
> > +     * doesn't match.
> > +     */
> > +    if ((in->flags & 0x1) != (region->flags & 0x1)) {
> > +        return CXL_MBOX_UNSUPPORTED;
> > +    }
> > +
> > +    if (in->reg_id >= DCD_MAX_NUM_REGION) {
> > +        return CXL_MBOX_UNSUPPORTED;
> > +    }
> > +
> > +    /* Check that no extents are in the region being reconfigured */
> > +    if (!bitmap_empty(region->blk_bitmap, region->len / 
> > region->block_size)) {
> > +        return CXL_MBOX_UNSUPPORTED;
> > +    }
> > +
> > +    /* Check that new block size is supported */
> > +    if (!test_bit(BIT((int) log2(in->block_sz)),
> > +                  &region->supported_blk_size_bitmask)) {
> > +        return CXL_MBOX_INVALID_INPUT;
> > +    }  
> 
> This does not work: test_bit works on unsigned long, while
> supported_blk_size_bitmask is uint64_t.
> 
> Why so funky? what is wrong with:
> 
> if (!(BIT_ULL(log2(in->block_sz)) & region->supported_blk_size_bitmask))
> 
> And BTW why cast to int here?

Change looks fine to me, so I'll prepare an updated set with this
and the missing semi colon.  Anisa if you can have a look at this
that would be great. 

Sorry I seem to have missed Anisa off the cc for this!

Jonathan