Re: [RFC PATCH 06/25] hw/cxl/device: Add device status (8.2.8.3)

[Jonathan Cameron]

On Mon, 16 Nov 2020 13:18:41 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> On 20-11-16 13:16:08, Jonathan Cameron wrote:
> > On Tue, 10 Nov 2020 21:47:05 -0800
> > Ben Widawsky <ben.widawsky@intel.com> wrote:
> >   
> > > This implements the CXL device status registers from 8.2.8.3.1 in the
> > > CXL 2.0 specification. It is capability ID 0001h.
> > > 
> > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>  
> > 
> > It does some other stuff it shouldn't as well.  Please tidy that up before
> > v2.  A few other passing comments inline.
> > 
> > Thanks,
> > 
> > Jonathan
> > 
> >   
> > > ---
> > >  hw/cxl/cxl-device-utils.c   | 45 +++++++++++++++++++++++++++++++++-
> > >  include/hw/cxl/cxl_device.h | 49 ++++++++++++-------------------------
> > >  2 files changed, 60 insertions(+), 34 deletions(-)
> > > 
> > > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> > > index a391bb15c6..78144e103c 100644
> > > --- a/hw/cxl/cxl-device-utils.c
> > > +++ b/hw/cxl/cxl-device-utils.c
> > > @@ -33,6 +33,42 @@ static uint64_t caps_reg_read(void *opaque, hwaddr 
> > > offset, unsigned size)
> > >      return ldn_le_p(cxl_dstate->caps_reg_state + offset, size);
> > >  }
> > >  
> > > +static uint64_t dev_reg_read(void *opaque, hwaddr offset, unsigned size)
> > > +{
> > > +    uint64_t retval = 0;  
> > 
> > Doesn't seem to be used.
> >   
> 
> It's required for ldn_le_p, or did you mean something else?

Nope just failed to notice that use. oops

> 
> > > +  
> > 
> > Perhaps break the alignment check out to a utility function given this 
> > sanity check
> > is same as in previous patch.
> >   
> > > +    switch (size) {
> > > +    case 4:
> > > +        if (unlikely(offset & (sizeof(uint32_t) - 1))) {
> > > +            qemu_log_mask(LOG_UNIMP, "Unaligned register read\n");
> > > +            return 0;
> > > +        }
> > > +        break;
> > > +    case 8:
> > > +        if (unlikely(offset & (sizeof(uint64_t) - 1))) {
> > > +            qemu_log_mask(LOG_UNIMP, "Unaligned register read\n");
> > > +            return 0;
> > > +        }
> > > +        break;
> > > +    }
> > > +
> > > +    return ldn_le_p(&retval, size);
> > > +}