Re: [PATCH] hw/pci-bridge/pci_expander_bridge: Fix HDM passthrough condition

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Mon, 7 Apr 2025 02:59:20 +0000
"Zhijian Li (Fujitsu)" <lizhijian@fujitsu.com> wrote:

> Ping

Sorry, I wrote half a reply but then lost it before sending
- was still in my drafts :(

> 
> Only if (dsp_count==1 && hdm_for_passthrough==true), the QEMU shouldn't implement
> the HDM decodder for the Host-bridge.

HDM for pass through means that we do have decoders even for pass through
not that we do not.  The name could be better and the code flow
perhaps simpler.

> 
> But previous code didn't follow this.

> Thanks
> Zhijian
> 
> On 23/03/2025 16:04, Li Zhijian wrote:
> > Reverse the logical condition for HDM passthrough support in
> > pci_expander_bridge. This patch ensures the HDM passthrough condition
> > is evaluated only when hdm_for_passthrough is set to true, aligning
> > behavior with intended semantics and comments.
> > 
> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> > ---
> > 
> > This change corrects what appears to be a previous mistake in logic
> > regarding HDM passthrough conditions.
> > ---
> >   hw/pci-bridge/pci_expander_bridge.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > index 3396ab4bdd..25f8922d76 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -307,7 +307,7 @@ static void pxb_cxl_dev_reset(DeviceState *dev)
> >        * The CXL specification allows for host bridges with no HDM decoders
> >        * if they only have a single root port.
> >        */
> > -    if (!PXB_CXL_DEV(dev)->hdm_for_passthrough) {

This makes us only check the number of ports if we are not
providing hdm decoders for passthrough ports.

If we are providing HDM Decoders anyway we don't care how many
ports there are so the 0 value is fine.

> > +    if (PXB_CXL_DEV(dev)->hdm_for_passthrough) {
> >           dsp_count = pcie_count_ds_ports(hb->bus);
> >       }
> >       /* Initial reset will have 0 dsp so wait until > 0 *