All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tushar Dave <tdave@nvidia.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Alex Williamson <alex@shazbot.org>,
	qemu-devel@nongnu.org, jgg@nvidia.com, skolothumtho@nvidia.com,
	qemu-arm@nongnu.org, peter.maydell@linaro.org, mst@redhat.com,
	marcel.apfelbaum@gmail.com, devel@edk2.groups.io
Subject: Re: [RFC PATCH v2 0/5] hw/pci, hw/arm/virt: fixed PCI BAR placement
Date: Wed, 2 Sep 2026 11:30:33 -0500	[thread overview]
Message-ID: <37f31842-e79a-4089-82ad-8003fd47fdb8@nvidia.com> (raw)
In-Reply-To: <ape0Mbd7Lt4KS-wN@sirius.home.kraxel.org>



On 9/2/2026 1:15 AM, Gerd Hoffmann wrote:
>   Hi,
> 
>>> And, yes, the logic to match entries in the fw_cfg file with the correct
>>> device using vendor and device id looks somewhat fragile to me too.
>>>
>>> Existing code in qemu+firmware (for example bootorder) uses the location
>>> in the physical device tree to identify devices, like this:
>>>
>>> /pci@i0cf8/pci-bridge@3/*@0/*@0/*@0,0
>>>  ^^^^^^^^^                             pcie root bus
>>>            ^^^^^^^^^^^^                pcie root port @ slot 3
>>>                         ^^^            virtio-scsi-pci @ slot 0
>>>                             ^^^        scsi controller bus #0
>>>                                 ^^^^^  scsi device target 0, lun 0
>>
>> Good point but the problem is CheckDevice()'s own signature, which is
>> fixed by UEFI PI spec (only passes
>> VendorId/DeviceId/RevisionId/SubsystemVendorId/SubsystemDeviceId). Even
>> though the path exists internally, the standard protocol interface
>> doesn't pass it to the callback.
> 
> Hmm, yes.  Seems to be designed to apply quirks to device classes, not
> individual devices.
> 
> Also note that OVMF already has an incompatible pci device driver and
> there can be only one instance, so the code must be merged into the
> existing driver instead of adding a second.

I checked OvmfPkg/IncompatiblePciDeviceSupportDxe -- its CheckDevice()
is unconditional, it returns the same 64-bit-MMIO-preference descriptor
for every device regardless of VendorId/DeviceId. Merging Fixed BAR
design in would make it a simple dispatch: if the device has an entry in
the fw_cfg blob we export, return our descriptor; otherwise fall through
to the existing behavior unchanged. Does that match what you had in
mind, or is there a different integration point you'd prefer?

> 
>> Therefore, we prepare the blob entries
>> in the same order PciBusDxe discovers devices, so matching by VID:DID
>> inherently works.
> 
> Question is whenever we want have that edk2 limitation and the knowledge
> about edk2 internals (pci scan order) encoded in the qemu <-> firmware
> protocol.  I think it makes sense to (additionally) pass the complete
> device path even if the current edk2 implementation doesn't use it, so
> we have the option to improve things later on without having to change
> the qemu <-> firmware protocolS for that.

I see your point. Sure thing, I'll add it.

> 
>>> I can see that allowing fixed and non-fixed bars mix is much harder to
>>> handle.  Do we need to ask the user to manually set that though?  I'd
>>> prefer pci devices propagating automatically to the parent bus that they
>>> have fixed bars and additional constrains apply.
>>
>> I looked at this again, and technically nothing actually needs the flag
>> to exist. The real reason I kept it is closer to a usability one; it's
>> meant to be a visible signal in the launch script itself, so anyone
>> reading or writing the qemu command line sees up front that every device
>> under that root port is expected to have pci-bars= configured, rather
>> than that requirement only surfacing as a runtime error if something's
>> missing.
> 
> I'm not sure how much of a usability win that actually is, if you forget
> to set the flag you still get a runtime error.

Fair point. I will drop 'fixed-bar=on' from RP property.

Thanks.
-Tushar
> 
> In general I like things which can be done automatically actually happen
> automatically as this simplifies things for the user in most cases.
> 
>>> Also: if the main use case for this is to map vfio devices with guest
>>> physical address == host physical address, is there a need to specify
>>> this manually at all?  Shouldn't we have a 'vfio-pci-fixed' device which
>>> handles this automatically?
>>
>> VFIO GPA == HPA is the primary motivation, but I don't think fixed-bar
>> should be tied to VFIO or automatically derive guest addresses from the
>> host.
> 
> Why not?  It is a great usability improvement IMHO.
> 
>> For the VFIO use case, the admin can choose to specify the host
>> BAR addresses as the fixed-bar configuration to get GPA == HPA, but the
>> mechanism itself doesn't assume or enforce that -- the desired guest
>> layout isn't always just a copy of the host's, so having fixed-bar
>> auto-derive it on its own would be incorrect in some cases, not just
>> less general.
> 
> You still can have fixed-bar-<nr>=<addr> properties to override the
> auto-discovered address for some or all pci bars.
> 
>> The mechanism remains a generic way to explicitly specify
>> PCI BAR addresses.
> 
> Yes, the code which creates the fw_cfg files is generic and it makes
> sense to have that in the core pci code, so it can be used for every pci
> device.
> 
> Nevertheless I'd tend to only expose the properties for devices where an
> actual use case exists.  Which is obviously vfio-pci(-fixed).  Also
> pci-testdev for development / testing / CI.  I can't see much beyond
> that though.
> 
> take care,
>   Gerd


  parent reply	other threads:[~2026-09-02 16:31 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27  0:40 [RFC PATCH v2 0/5] hw/pci, hw/arm/virt: fixed PCI BAR placement Tushar Dave
2026-08-27  0:40 ` [RFC PATCH v2 1/5] hw/pci: add fixed-bar and pci-bars properties Tushar Dave
2026-08-27  0:40 ` [RFC PATCH v2 2/5] pci: add validation for fixed BAR configuration Tushar Dave
2026-08-27  0:40 ` [RFC PATCH v2 3/5] pci: add fixed BAR fw_cfg blob export Tushar Dave
2026-08-27  0:40 ` [RFC PATCH v2 4/5] hw/arm/virt: export fixed BAR metadata via fw_cfg Tushar Dave
2026-08-27  0:40 ` [RFC PATCH v2 5/5] hw/arm/virt: add highmem-mmio-base property Tushar Dave
2026-08-27  7:18 ` [RFC PATCH v2 0/5] hw/pci, hw/arm/virt: fixed PCI BAR placement Gerd Hoffmann
2026-08-27 13:47   ` Alex Williamson
2026-08-27 14:38     ` [edk2-devel] " Ard Biesheuvel
2026-08-28 15:49       ` Tushar Dave
2026-08-31 13:42         ` Gerd Hoffmann
2026-09-01 21:58           ` Tushar Dave
2026-08-31 13:24     ` Gerd Hoffmann
2026-09-01 22:27       ` Tushar Dave
2026-09-02  6:15         ` Gerd Hoffmann
2026-09-02 15:24           ` Alex Williamson
2026-09-03  9:34             ` Gerd Hoffmann
2026-09-02 16:30           ` Tushar Dave [this message]
2026-09-03  9:47             ` Gerd Hoffmann
2026-09-04 13:46               ` Tushar Dave

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37f31842-e79a-4089-82ad-8003fd47fdb8@nvidia.com \
    --to=tdave@nvidia.com \
    --cc=alex@shazbot.org \
    --cc=devel@edk2.groups.io \
    --cc=jgg@nvidia.com \
    --cc=kraxel@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=skolothumtho@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.