Re: [PATCH 2/2] hw/pxb-cxl: Support passthrough HDM Decoders unless overridden

[Jonathan Cameron]

On Fri, 27 Jan 2023 17:02:36 +0000
Fan Ni <fan.ni@samsung.com> wrote:

> On Fri, Jan 27, 2023 at 10:01:49AM +0000, Jonathan Cameron wrote:
> 
> > On Thu, 26 Jan 2023 21:57:35 +0000
> > Fan Ni <fan.ni@samsung.com> wrote:
> >   
> > > On Wed, Jan 25, 2023 at 03:27:03PM +0000, Jonathan Cameron wrote:
> > >   
> > > > The CXL r3.0 specification allows for there to be no HDM decoders on CXL
> > > > Host Bridges if they have only a single root port. Instead, all accesses
> > > > directed to the host bridge (as specified in CXL Fixed Memory Windows)
> > > > are assumed to be routed to the single root port.
> > > > 
> > > > Linux currently assumes this implementation choice. So to simplify 
> > > > testing,
> > > > make QEMU emulation also default to no HDM decoders under these 
> > > > particular
> > > > circumstances, but provide a hdm_for_passthrough boolean option to have
> > > > HDM decoders as previously.
> > > > 
> > > > Technically this is breaking backwards compatibility, but given the only
> > > > known software stack used with the QEMU emulation is the Linux kernel
> > > > and this configuration did not work before this change, there are
> > > > unlikely to be any complaints that it now works. The option is retained
> > > > to allow testing of software that does allow for these HDM decoders to 
> > > > exist,
> > > > once someone writes it.
> > > > 
> > > > Reported-by: Fan Ni <fan.ni@samsung.com>
> > > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> >   
> > > >      pxb_dev_realize_common(dev, CXL, errp);
> > > > -    pxb_dev_reset(DEVICE(dev));
> > > > +    pxb_cxl_dev_reset(DEVICE(dev));
> > > >  }
> > > >  
> > > > +static Property pxb_cxl_dev_properties[] = {
> > > > +    /* Note: 0 is not a legal PXB bus number. */
> > > > +    DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0),
> > > > +    DEFINE_PROP_UINT16("numa_node", PXBDev, numa_node, 
> > > > NUMA_NODE_UNASSIGNED),
> > > > +    DEFINE_PROP_BOOL("bypass_iommu", PXBDev, bypass_iommu, false),
> > > > +    DEFINE_PROP_BOOL("hdm_for_passthrough", PXBDev, 
> > > > hdm_for_passthrough, false),    
> > > when setting hdm_for_passthrough to true at the qemu command line, we
> > > will see the segfault issue as before. I think this is expected as it
> > > is the logic in cxl_cfmws_find_device. Wondering if there will be
> > > following fixes to handle the case when hdm_for_passthrough is true.  
> > 
> > Absolutely, I'd expect a kernel fix for that case, but it's probably not
> > high priority for anyone given we don't yet have any hardware that does that
> > (as far as I know anyway!)
> > 
> > I wanted to keep the control here to make that easy to test when we do
> > have the fix in place.
> > Jonathan  
> Make sense!
> 
> Reviwed-by: Fan Ni <fan.ni@samsung.com>
> Tested-by: Fan Ni <fan.ni@samsung.com>

Thanks!

Typo in Reviewed-by If this version gets picked up hopefully Michael can
fix whilst applying. If I do a v2 for other feedback I'll fix it.

Jonathan