Re: [PATCH 4/4] hw/pci: Ensure pci_add_capability() is called before device is realized
On 22/3/23 09:52, Philippe Mathieu-Daudé wrote:
On 22/3/23 03:18, Michael S. Tsirkin wrote:
On Tue, Mar 14, 2023 at 12:14:35PM +0100, Philippe Mathieu-Daudé wrote:
PCI capabilities can't appear magically at runtime.
Guests aren't expecting that. Assert all capabilities
are added _before_ a device instance is realized.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/pci/pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ac41fcbf6a..ed60b352e4 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2397,7 +2397,7 @@ static void pci_del_option_rom(PCIDevice *pdev)
* On success, pci_add_capability() returns a positive value
* that the offset of the pci capability.
* On failure, it sets an error and returns a negative error
- * code.
+ * code. @pdev must be unrealized.
*/
int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size,
@@ -2406,6 +2406,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t
cap_id,
uint8_t *config;
int i, overlapping_cap;
+ assert(!DEVICE(pdev)->realized);
+
if (!offset) {
offset = pci_find_space(pdev, size);
/* out of PCI config space is programming error */
Fails in CI:
https://gitlab.com/mstredhat/qemu/-/jobs/3976974199
qemu-system-i386: ../hw/pci/pci.c:2409: pci_add_capability: Assertion
`!DEVICE(pdev)->realized' failed.
Broken pipe
../tests/qtest/libqtest.c:193: kill_qemu() detected QEMU death from
signal 6 (Aborted) (core dumped)
TAP parsing error: Too few tests run (expected 49, got 40)
(test program exited with status code -6)
Thanks for testing!
Likely the AMD-Vi device, see on the cover this series is
Based-on: <20230313153031.86107-1-phi...@linaro.org>
"hw/i386/amd_iommu: Orphanize & QDev cleanups"
https://lore.kernel.org/qemu-devel/20230313153031.86107-1-phi...@linaro.org/
I confirm this is the AMD-Vi device, so you are missing the
previous (based-on) series:
#1 0x102d4e5b0 in pci_add_capability pci.c:2354
#2 0x102d2ff28 in msi_init msi.c:227
#3 0x10371a340 in amdvi_sysbus_realize amd_iommu.c:1553
#4 0x1037194e8 in x86_iommu_realize x86-iommu.c:124
#5 0x10409db88 in device_set_realized+0x788
(qemu-system-i386:arm64+0x101d91b88)
#6 0x1040cb248 in property_set_bool+0x2a0
(qemu-system-i386:arm64+0x101dbf248)
#7 0x1040c31f4 in object_property_set+0x4bc
(qemu-system-i386:arm64+0x101db71f4)
#8 0x1040d9990 in object_property_set_qobject+0x38
(qemu-system-i386:arm64+0x101dcd990)
#9 0x1040c40f8 in object_property_set_bool+0xfc
(qemu-system-i386:arm64+0x101db80f8)
#10 0x10409639c in qdev_realize+0x3bc
(qemu-system-i386:arm64+0x101d8a39c)
#11 0x10334f8e8 in qdev_device_add_from_qdict qdev-monitor.c:714
#12 0x103352114 in qdev_device_add qdev-monitor.c:733
#13 0x10337906c in device_init_func vl.c:1140
#14 0x104a84200 in qemu_opts_foreach qemu-option.c:1135
#15 0x103364fcc in qemu_create_cli_devices vl.c:2541
#16 0x103364ab8 in qmp_x_exit_preconfig vl.c:2609
#17 0x10336c0dc in qemu_init vl.c:3611
#18 0x1040812e4 in main main.c:47
#19 0x1a025be4c ()
Due to the required base series, this is 8.1 material.
Re: [PATCH 4/4] hw/pci: Ensure pci_add_capability() is called before device is realized
On 22/3/23 03:18, Michael S. Tsirkin wrote:
On Tue, Mar 14, 2023 at 12:14:35PM +0100, Philippe Mathieu-Daudé wrote:
PCI capabilities can't appear magically at runtime.
Guests aren't expecting that. Assert all capabilities
are added _before_ a device instance is realized.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/pci/pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ac41fcbf6a..ed60b352e4 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2397,7 +2397,7 @@ static void pci_del_option_rom(PCIDevice *pdev)
* On success, pci_add_capability() returns a positive value
* that the offset of the pci capability.
* On failure, it sets an error and returns a negative error
- * code.
+ * code. @pdev must be unrealized.
*/
int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size,
@@ -2406,6 +2406,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t *config;
int i, overlapping_cap;
+assert(!DEVICE(pdev)->realized);
+
if (!offset) {
offset = pci_find_space(pdev, size);
/* out of PCI config space is programming error */
Fails in CI:
https://gitlab.com/mstredhat/qemu/-/jobs/3976974199
qemu-system-i386: ../hw/pci/pci.c:2409: pci_add_capability: Assertion
`!DEVICE(pdev)->realized' failed.
Broken pipe
../tests/qtest/libqtest.c:193: kill_qemu() detected QEMU death from signal 6
(Aborted) (core dumped)
TAP parsing error: Too few tests run (expected 49, got 40)
(test program exited with status code -6)
Thanks for testing!
Likely the AMD-Vi device, see on the cover this series is
Based-on: <20230313153031.86107-1-phi...@linaro.org>
"hw/i386/amd_iommu: Orphanize & QDev cleanups"
https://lore.kernel.org/qemu-devel/20230313153031.86107-1-phi...@linaro.org/
Re: [PATCH 4/4] hw/pci: Ensure pci_add_capability() is called before device is realized
On Tue, Mar 14, 2023 at 12:14:35PM +0100, Philippe Mathieu-Daudé wrote:
> PCI capabilities can't appear magically at runtime.
> Guests aren't expecting that. Assert all capabilities
> are added _before_ a device instance is realized.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/pci/pci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index ac41fcbf6a..ed60b352e4 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2397,7 +2397,7 @@ static void pci_del_option_rom(PCIDevice *pdev)
> * On success, pci_add_capability() returns a positive value
> * that the offset of the pci capability.
> * On failure, it sets an error and returns a negative error
> - * code.
> + * code. @pdev must be unrealized.
> */
> int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> uint8_t offset, uint8_t size,
> @@ -2406,6 +2406,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
> uint8_t *config;
> int i, overlapping_cap;
>
> +assert(!DEVICE(pdev)->realized);
> +
> if (!offset) {
> offset = pci_find_space(pdev, size);
> /* out of PCI config space is programming error */
Fails in CI:
https://gitlab.com/mstredhat/qemu/-/jobs/3976974199
qemu-system-i386: ../hw/pci/pci.c:2409: pci_add_capability: Assertion
`!DEVICE(pdev)->realized' failed.
Broken pipe
../tests/qtest/libqtest.c:193: kill_qemu() detected QEMU death from signal 6
(Aborted) (core dumped)
TAP parsing error: Too few tests run (expected 49, got 40)
(test program exited with status code -6)
> --
> 2.38.1
[PATCH 4/4] hw/pci: Ensure pci_add_capability() is called before device is realized
PCI capabilities can't appear magically at runtime.
Guests aren't expecting that. Assert all capabilities
are added _before_ a device instance is realized.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/pci/pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index ac41fcbf6a..ed60b352e4 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2397,7 +2397,7 @@ static void pci_del_option_rom(PCIDevice *pdev)
* On success, pci_add_capability() returns a positive value
* that the offset of the pci capability.
* On failure, it sets an error and returns a negative error
- * code.
+ * code. @pdev must be unrealized.
*/
int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size,
@@ -2406,6 +2406,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t *config;
int i, overlapping_cap;
+assert(!DEVICE(pdev)->realized);
+
if (!offset) {
offset = pci_find_space(pdev, size);
/* out of PCI config space is programming error */
--
2.38.1
