Re: [PATCH 0/2] hw/pci-bridge: pci_expander_bridge: Fix wrong type and rework inheritance.

[Jonathan Cameron]

On Fri, 21 Apr 2023 09:59:57 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Fri, 21 Apr 2023 at 09:19, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Apr 20, 2023 at 03:27:48PM +0100, Jonathan Cameron wrote:  
> > > Peter Maydell highlighted an incorrect conversion to TYPE_PXB_DEVICE from
> > > a device that didn't have that a an ancestor type. PXB_DEV() used instead 
> > > of
> > > PXB_CXL_DEV()/
> > >
> > > CAFEAcA-+de+eeLCE4YsAw1O-Qyd_4W1Ra05mGDsU_-3a6d92qw@mail.gmail.com/">https://lore.kernel.org/qemu-devel/CAFEAcA-+de+eeLCE4YsAw1O-Qyd_4W1Ra05mGDsU_-3a6d92qw@mail.gmail.com/
> > >
> > > During the discussion it became clear that the inheritance of the various
> > > TYPE_PXB*_DEVICE was unusual. This patchset first provides the minimal
> > > fix then cleans up the inheritance of types based on functionality.
> > >
> > > There is also a rename to TYPE_PXB*_DEV to allow removal of some 
> > > boilerplate.
> > >
> > > Before this series
> > > TYPE_PXB_DEVICE, TYPE_PXB_PCIE_DEVICE and TYPE_PXB_CXL_DEVICE all
> > > had TYPE_PCI_DEVICE as their direct parent though they shared a common
> > > struct PXBDev for their state.  As a result this state contained
> > > some data that was irrelevant for some the types.
> > >
> > > This series changes to
> > > TYPE_PXB_CXL_DEV has a parent of TYPE_PXB_PCIE_DEV
> > > TYPE_PXB_PCIE_DEV has a parent of TYPE_PXB_DEV
> > > TYPE_PXB_DEV continues to have a parent of TYPE_PCI_DEVICE.
> > >
> > > Each of the TYPE_PXB*_DEV has a state structure adding those elements
> > > to their parent that they need. This also allowed dropping a wrapping
> > > structure for the CXL state as the PXBCXLDev structure already provides
> > > the equivalent grouping.
> > >
> > > Patches are similar to those posted in the thread but rebased on v8.0.0.  
> >
> > this conflicts with
> >     Revert "hw/pxb-cxl: Support passthrough HDM Decoders unless overridden"
> >
> > I think you acked that one?  
> 
> We should take one or the other, but not both. If this patchset
> is good then it's probably better to fix the bug rather than
> revert the feature, I think.

If it's easy to drop the revert that would be my preference.

If not, then I'm fine spinning a new version of that patch without
the bug (so with patch 1 of this squashed in).  Patch 2 is somewhat related
refactoring.  Not necessary to fix the issue even though it was motivated
by that bug.

Jonathan

> 
> -- PMM