Re: [PATCH v2] PCI/TPH: fold reserved completer encoding in get_rp_completer_type()

[Zhiping Zhang]

> > +static u8 tph_completer_type_fold(u8 comp)
> > +{
> > +     switch (comp) {
> > +     case PCI_EXP_DEVCAP2_TPH_COMP_TPH_ONLY:
> > +     case PCI_EXP_DEVCAP2_TPH_COMP_EXT_TPH:
> > +             return comp;
> > +     default:
> > +             return PCI_EXP_DEVCAP2_TPH_COMP_NONE;
> > +     }
> > +}
> > +
> >  /* Return device's Root Port completer capability */
> >  static u8 get_rp_completer_type(struct pci_dev *pdev)
> >  {
> > @@ -211,7 +226,8 @@ static u8 get_rp_completer_type(struct pci_dev *pdev)
> >       if (ret)
> >               return 0;
> >
> > -     return FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK, reg);
> > +     return tph_completer_type_fold(FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK,
> > +                                              reg));
>
> Is this just defensive programming, or is there some hardware out
> there that uses this reserved value?  It's kind of an ugly wart to add
> just for theoretical spec conformance testing.

Purely defensive -- no known RP presents the reserved 0b10 (Wei
confirms downthread). It was split out of my vfio/dma-buf TPH series
review [1], where the 0b10 passthrough was flagged. It matters because
the value flows through min(tph_req_type, rp_req_type) into the TPH
Requester Enable write, so a broken RP's reserved encoding could reach
hardware.

>
> If we really need it, I'd rather read something like this:
>
>     tph_comp = FIELD_GET(PCI_EXP_DEVCAP2_TPH_COMP_MASK, reg);
>     if (tph_comp == PCI_EXP_DEVCAP2_TPH_COMP_TPH_ONLY ||
>         tph_comp == PCI_EXP_DEVCAP2_TPH_COMP_EXT_TPH)
>             return tph_comp;
>
>     return 0;

Will do for v3, plus the comment Wei asked for. Note that series [1]
adds a second reader of this field (pcie_tph_completer_type()) needing
the same fold -- I can keep one helper or repeat the inline check.

Thanks,
Zhiping

[1] https://lore.kernel.org/linux-pci/20260709132602.6a3fb084@xxxxxxxxxxx/