Re: [PATCH v4] x86/hvmloader: select xenpci MMIO BAR UC or WB MTRR cache attribute

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jun 12, 2025 at 03:12:03PM +0000, Tu Dinh wrote:
> On 12/06/2025 16:57, Roger Pau Monné wrote:
> > On Wed, Jun 11, 2025 at 07:26:06PM +0200, Anthony PERARD wrote:
> >> On Tue, Jun 10, 2025 at 06:29:30PM +0200, Roger Pau Monne wrote:
> >>> @@ -271,6 +279,44 @@ void pci_setup(void)
> >>>               if ( bar_sz == 0 )
> >>>                   continue;
> >>>
> >>> +            if ( !xenpci_bar_uc &&
> >>> +                 ((bar_data & PCI_BASE_ADDRESS_SPACE) ==
> >>> +                   PCI_BASE_ADDRESS_SPACE_MEMORY) &&
> >>> +                 vendor_id == 0x5853 &&
> >>> +                 (device_id == 0x0001 || device_id == 0x0002) )
> >>
> >> We don't have defines for 0x5853 in the tree (and those device_id)?
> >> Maybe introduce at least one for the vendor_id:
> >>
> >> These two names are use by QEMU, OVMF, Linux, for example.
> >>
> >> #define PCI_VENDOR_ID_XEN           0x5853
> >> #define PCI_DEVICE_ID_XEN_PLATFORM  0x0001
> >>
> >> There's even PCI_DEVICE_ID_XEN_PLATFORM_XS61 in Linux
> >
> > You mean in the public headers?
> >
> > For Device IDs we have ranges allocated to downstream vendors, I'm not
> > sure we want to keep track of whatever IDs they use for their devices.
> > OTOH, not tracking those IDs here means OSes are likely to miss
> > additions of new Xen platform PCI devices?
> >
> 
> The devices starting from ID c000 are supposed to be xen_pvdevice, which
> are separate device types that work differently from Xen platform PCI
> devices. So I think you only need to check for
> PCI_DEVICE_ID_XEN_PLATFORM{,_XS61} unless another platform PCI ID gets
> defined some day.

It's not the check here, but rather what would be defined in the newly
added header what I don't know.

Even if not used here, should the other PCI device IDs be listed
together with the platform PCI device ones?

Thanks, Roger.