Re: [PATCH-for-10.0 0/8] hw/boards: Remove legacy MachineClass::pci_allow_0_address flag

[Philippe Mathieu-Daudé]

On 25/11/24 15:14, Peter Maydell wrote:
> On Mon, 25 Nov 2024 at 14:06, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > This series aims to remove a legacy field from
> > MachineClass.
> >
> > Rather than a global exposed to all machines,
> > use a pci-bus specific flag on each machine
> > requiering it.
>
> Should this be a property of the PCI controller, rather
> than on the PCI bus? Presumably on the machines that
> don't allow a 0 PCI BAR address this happens because the
> PCI controller refuses to map BARs at that address.
>
> TBH the commit message for e402463073 suggests to me
> that "allow address zero" should be the default and
> either specific machines should forbid it or else we
> should figure out what goes wrong with them, if the
> problem is caused by some bug in QEMU. The commit message's
> mention of "fix PCI memory priorities" suggests to me
> that this is a QEMU bug, and that it ought to be possible
> to have the machine set up such that you *can* map the
> BAR at address 0, it's merely invisible to the guest because
> some other machine devices have higher priority and are
> visible "on top" of it instead.

You are probably right, the following comment ...:

 pcibus_t pci_bar_address(PCIDevice *d,
                          int reg, uint8_t type, pcibus_t size)
 {
     ...
     /* NOTE: we do not support wrapping */
     /* XXX: as we cannot support really dynamic
        mappings, we handle specific values as invalid
        mappings. */
     if (last_addr <= new_addr || last_addr == PCI_BAR_UNMAPPED ||
         (!allow_0_address && new_addr == 0)) {
         return PCI_BAR_UNMAPPED;
     }

... is from 20 years ago at the beginning of PCI in QEMU, commit
0ac32c8375 ("PCI interrupt support - PCI BIOS interrupt remapping
- more accurate memory mapping - 'info pci' monitor command") which
suggest the implementation is incomplete here.