Decouple Xen-HVM from PIIX

[PATCH 1/2] hw/i386/xen/xen-hvm: Allow for stubbing xen_set_pci_link_route()

Posted by Bernhard Beschow 3 years, 7 months ago

The only user of xen_set_pci_link_route() is
xen_piix_pci_write_config_client() which implements PIIX-specific logic in
the xen namespace. This makes xen-hvm depend on PIIX which could be
avoided if xen_piix_pci_write_config_client() was implemented in PIIX. In
order to do this, xen_set_pci_link_route() needs to be stubbable which
this patch addresses.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/xen/xen-hvm.c       | 7 ++++++-
 include/hw/xen/xen.h        | 1 +
 include/hw/xen/xen_common.h | 6 ------
 stubs/xen-hw-stub.c         | 5 +++++
 4 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 0731f70410..204fda7949 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -161,11 +161,16 @@ void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
         }
         v &= 0xf;
         if (((address + i) >= PIIX_PIRQCA) && ((address + i) <= PIIX_PIRQCD)) {
-            xen_set_pci_link_route(xen_domid, address + i - PIIX_PIRQCA, v);
+            xen_set_pci_link_route(address + i - PIIX_PIRQCA, v);
         }
     }
 }
 
+int xen_set_pci_link_route(uint8_t link, uint8_t irq)
+{
+    return xendevicemodel_set_pci_link_route(xen_dmod, xen_domid, link, irq);
+}
+
 int xen_is_pirq_msi(uint32_t msi_data)
 {
     /* If vector is 0, the msi is remapped into a pirq, passed as
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 0f9962b1c1..13bffaef53 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -21,6 +21,7 @@ extern enum xen_mode xen_mode;
 extern bool xen_domid_restrict;
 
 int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
+int xen_set_pci_link_route(uint8_t link, uint8_t irq);
 void xen_piix3_set_irq(void *opaque, int irq_num, int level);
 void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 179741ff79..77ce17d8a4 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -316,12 +316,6 @@ static inline int xen_set_pci_intx_level(domid_t domid, uint16_t segment,
                                              device, intx, level);
 }
 
-static inline int xen_set_pci_link_route(domid_t domid, uint8_t link,
-                                         uint8_t irq)
-{
-    return xendevicemodel_set_pci_link_route(xen_dmod, domid, link, irq);
-}
-
 static inline int xen_inject_msi(domid_t domid, uint64_t msi_addr,
                                  uint32_t msi_data)
 {
diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
index 15f3921a76..743967623f 100644
--- a/stubs/xen-hw-stub.c
+++ b/stubs/xen-hw-stub.c
@@ -23,6 +23,11 @@ void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
 {
 }
 
+int xen_set_pci_link_route(uint8_t link, uint8_t irq)
+{
+    return -1;
+}
+
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
 {
 }
-- 
2.36.1

Re: [PATCH 1/2] hw/i386/xen/xen-hvm: Allow for stubbing xen_set_pci_link_route()

Posted by Laurent Vivier 3 years, 7 months ago

Le 26/06/2022 à 11:46, Bernhard Beschow a écrit :
> The only user of xen_set_pci_link_route() is
> xen_piix_pci_write_config_client() which implements PIIX-specific logic in
> the xen namespace. This makes xen-hvm depend on PIIX which could be
> avoided if xen_piix_pci_write_config_client() was implemented in PIIX. In
> order to do this, xen_set_pci_link_route() needs to be stubbable which
> this patch addresses.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i386/xen/xen-hvm.c       | 7 ++++++-
>   include/hw/xen/xen.h        | 1 +
>   include/hw/xen/xen_common.h | 6 ------
>   stubs/xen-hw-stub.c         | 5 +++++
>   4 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
> index 0731f70410..204fda7949 100644
> --- a/hw/i386/xen/xen-hvm.c
> +++ b/hw/i386/xen/xen-hvm.c
> @@ -161,11 +161,16 @@ void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
>           }
>           v &= 0xf;
>           if (((address + i) >= PIIX_PIRQCA) && ((address + i) <= PIIX_PIRQCD)) {
> -            xen_set_pci_link_route(xen_domid, address + i - PIIX_PIRQCA, v);
> +            xen_set_pci_link_route(address + i - PIIX_PIRQCA, v);
>           }
>       }
>   }
>   
> +int xen_set_pci_link_route(uint8_t link, uint8_t irq)
> +{
> +    return xendevicemodel_set_pci_link_route(xen_dmod, xen_domid, link, irq);
> +}
> +
>   int xen_is_pirq_msi(uint32_t msi_data)
>   {
>       /* If vector is 0, the msi is remapped into a pirq, passed as
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index 0f9962b1c1..13bffaef53 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -21,6 +21,7 @@ extern enum xen_mode xen_mode;
>   extern bool xen_domid_restrict;
>   
>   int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
> +int xen_set_pci_link_route(uint8_t link, uint8_t irq);
>   void xen_piix3_set_irq(void *opaque, int irq_num, int level);
>   void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len);
>   void xen_hvm_inject_msi(uint64_t addr, uint32_t data);
> diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
> index 179741ff79..77ce17d8a4 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -316,12 +316,6 @@ static inline int xen_set_pci_intx_level(domid_t domid, uint16_t segment,
>                                                device, intx, level);
>   }
>   
> -static inline int xen_set_pci_link_route(domid_t domid, uint8_t link,
> -                                         uint8_t irq)
> -{
> -    return xendevicemodel_set_pci_link_route(xen_dmod, domid, link, irq);
> -}
> -
>   static inline int xen_inject_msi(domid_t domid, uint64_t msi_addr,
>                                    uint32_t msi_data)
>   {
> diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
> index 15f3921a76..743967623f 100644
> --- a/stubs/xen-hw-stub.c
> +++ b/stubs/xen-hw-stub.c
> @@ -23,6 +23,11 @@ void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int len)
>   {
>   }
>   
> +int xen_set_pci_link_route(uint8_t link, uint8_t irq)
> +{
> +    return -1;
> +}
> +
>   void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
>   {
>   }

Applied to my trivial-patches branch.

Thanks,
Laurent

Re: [PATCH 1/2] hw/i386/xen/xen-hvm: Allow for stubbing xen_set_pci_link_route()

Posted by Durrant, Paul 3 years, 7 months ago

On 26/06/2022 10:46, Bernhard Beschow wrote:
> The only user of xen_set_pci_link_route() is
> xen_piix_pci_write_config_client() which implements PIIX-specific logic in
> the xen namespace. This makes xen-hvm depend on PIIX which could be
> avoided if xen_piix_pci_write_config_client() was implemented in PIIX. In
> order to do this, xen_set_pci_link_route() needs to be stubbable which
> this patch addresses.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Reviewed-by: Paul Durrant <paul@xen.org>