On Mon, 25 Jul 2022 16:59:21 +0300 Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru> wrote:
> On 7/20/22 14:04, Daniel P. Berrangé wrote: > > On Wed, Jul 20, 2022 at 02:00:16PM +0300, Roman Kagan wrote: > >> On Wed, Jul 20, 2022 at 11:44:26AM +0100, Daniel P. Berrangé wrote: > >>> On Wed, Jul 20, 2022 at 01:25:55PM +0300, Roman Kagan wrote: > >>>> It's possible to create non-working configurations by attaching a device > >>>> to a derivative of PCIe slot (pcie-root-port, ioh3420, etc) and > >>>> specifying a slot number other that zero, e.g.: > >>>> > >>>> -device pcie-root-port,id=s0,... \ > >>>> -device virtio-blk-pci,bus=s0,addr=4,... > >>>> > >>>> Make QEMU reject such configurations and only allow addr=0 on the > >>>> secondary bus of a PCIe slot. > >>> > >>> What do you mean by 'non-working' in this case. The guest OS boots > >>> OK, but I indeed don't see the device in the guest, but IIUC it was > >>> said that was just because Linux doesn't scan for a non-zero slot. > >> > >> Right. I don't remember if it was Linux or firmware or both but indeed > >> at least Linux guests don't see devices if attached to a PCIe slot at > >> addr != 0. (Which is kinda natural for a thing called "slot", isn't it?) > > > > I vaguely recall there was an option to tell linux to scan all slots, > > not just slot 0, not sure if that's applicable here. > > > >> > >>> That wouldn't be a broken config from QEMU's POV though, merely a > >>> guest OS limitation ? > >> > >> Strictly speaking it wouldn't, indeed. But we've had created such a > >> configuration (due to a bug in our management layer) and spent > >> non-negligible time trying to figure out why the attached device didn't > >> appear in the guest. So I thought it made sense to reject a > >> configuration which is known to confuse guests. Doesn't it? > > > > If a configuration is a permissible per the hardware design / spec, then > > QEMU should generally allow it. We don't want to constrain host side > > configs based on the current limitations of guest OS whose behaviour can > > change over time, or where a different guest OS may have a different POV. > > > > If I understand correctly further answers the configration that we try to > forbid is not permissible by PCIe spec. So seems valid to forbid it. We still > need to mention specification in commit message and in the comment. > > If we still afraid to forbid at once that invalid configuration that was > previously allowed, may be we can proceed with some of the following: > > 1. Make a deprecation period of three releases and print only warning during > this period. And forbid the invalid configuration three releases later. Still > I'm not sure that someone will see these warnings in logs.. > > 2. Make a boolean config option, like CONFIG_PCIE_STRICT, which forbids > invalid configurations. This way we keep default behavior, that allows to > test something unusual, but add an option that we can use for production > solution where it's important to reduce number of possibilities to break the > VM. > > What do you think? Given that non zero slots are used only on broken hardware/firmware and it's just workaround for those in linux kernel. I wouldn't bother with 1 or 2 (I think just a note on change log should be sufficient)