[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 13/15] hw/pci: Determine if rombar is explicitly enabled
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v6 13/15] hw/pci: Determine if rombar is explicitly enabled |
Date: |
Wed, 21 Feb 2024 09:15:06 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> vfio determines if rombar is explicitly enabled by inspecting QDict.
> Inspecting QDict is not nice because QDict is untyped and depends on the
> details on the external interface. Add an infrastructure to determine if
> rombar is explicitly enabled to hw/pci.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> include/hw/pci/pci_device.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
> index ca151325085d..c4fdc96ef50d 100644
> --- a/include/hw/pci/pci_device.h
> +++ b/include/hw/pci/pci_device.h
> @@ -205,6 +205,11 @@ static inline uint16_t pci_get_bdf(PCIDevice *dev)
> return PCI_BUILD_BDF(pci_bus_num(pci_get_bus(dev)), dev->devfn);
> }
>
> +static inline bool pci_rom_bar_explicitly_enabled(PCIDevice *dev)
> +{
> + return dev->rom_bar && dev->rom_bar != -1;
Compares signed with unsigned: rom_bar is uint32_t, -1 is int.
If int is at most 32 bits, the comparison converts -1 to
(uint32_t)0xffffffff.
If int is wider, it converts dev->rom_bar to int without changing its
value. In particular, it converts the default value 0xffffffff (written
as -1 in the previous patch) to (int)0xffffffff. Oops.
Best not to mix signed and unsigned in comparisons. Easy enough to
avoid here.
Still, we don't actually test "rom_bar has not been set". We test
"rom_bar has the default value". Nothing stops a user from passing
rombar=0xffffffff to -device. See my review of the next patch.
> +}
> +
> static inline void pci_set_power(PCIDevice *pci_dev, bool state)
> {
> /*
- [PATCH v6 06/15] hw/pci: Rename has_power to enabled, (continued)
- [PATCH v6 06/15] hw/pci: Rename has_power to enabled, Akihiko Odaki, 2024/02/20
- [PATCH v6 08/15] pcie_sriov: Reuse SR-IOV VF device instances, Akihiko Odaki, 2024/02/20
- [PATCH v6 11/15] pcie_sriov: Register VFs after migration, Akihiko Odaki, 2024/02/20
- [PATCH v6 10/15] pcie_sriov: Remove num_vfs from PCIESriovPF, Akihiko Odaki, 2024/02/20
- [PATCH v6 12/15] hw/pci: Use -1 as a default value for rombar, Akihiko Odaki, 2024/02/20
- [PATCH v6 09/15] pcie_sriov: Release VFs failed to realize, Akihiko Odaki, 2024/02/20
- [PATCH v6 07/15] pcie_sriov: Do not manually unrealize, Akihiko Odaki, 2024/02/20
- [PATCH v6 13/15] hw/pci: Determine if rombar is explicitly enabled, Akihiko Odaki, 2024/02/20
- Re: [PATCH v6 13/15] hw/pci: Determine if rombar is explicitly enabled,
Markus Armbruster <=
- [PATCH v6 14/15] vfio: Avoid inspecting option QDict for rombar, Akihiko Odaki, 2024/02/20
- [PATCH v6 15/15] hw/qdev: Remove opts member, Akihiko Odaki, 2024/02/20