Re: [Xen-devel] [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c

2018-04-24 Thread Julien Grall 

Hi,

On 04/23/2018 04:34 PM, Jan Beulich wrote:

On 21.02.18 at 22:46,  wrote:

Move the functions that reference x86 hvm data structures to its own
file.  Rename pci_clean_dpci_irqs to arch_pci_clean_irqs.

There is still one location in that file which references
arch.hvm_domain, but it is fine because ARM guest is HVM.

Signed-off-by: Wei Liu 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Jan Beulich 
Cc: Andrew Cooper 

ARM doesn't select HAS_PCI, that's why ARM build is not broken by
this. AIUI ARM will select HAS_PCI at some point, hence I only move
the x86 bits.


The fact that this change doesn't actually affect ARM is again an
indication that the decision whether the code you move here is
x86-specific depends on what parts of PCI pass-through ARM is
actually going to want to (re-)use. I'd again prefer to take that
decision either when ARM actually implements that, or based on
the ARM folks clearly foreseeing that this code is not coming
close to anything they may want use.


I can't see any use of that code on Arm. There are potential to move 
x86-ism outside, but this patch is already a good start.


I already gave my acked-by on a separate e-mail.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c

2018-04-23 Thread Jan Beulich 
>>> On 21.02.18 at 22:46,  wrote:
> Move the functions that reference x86 hvm data structures to its own
> file.  Rename pci_clean_dpci_irqs to arch_pci_clean_irqs.
> 
> There is still one location in that file which references
> arch.hvm_domain, but it is fine because ARM guest is HVM.
> 
> Signed-off-by: Wei Liu 
> ---
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> 
> ARM doesn't select HAS_PCI, that's why ARM build is not broken by
> this. AIUI ARM will select HAS_PCI at some point, hence I only move
> the x86 bits.

The fact that this change doesn't actually affect ARM is again an
indication that the decision whether the code you move here is
x86-specific depends on what parts of PCI pass-through ARM is
actually going to want to (re-)use. I'd again prefer to take that
decision either when ARM actually implements that, or based on
the ARM folks clearly foreseeing that this code is not coming
close to anything they may want use.

> @@ -853,7 +804,7 @@ int pci_release_devices(struct domain *d)
>  int ret;
>  
>  pcidevs_lock();
> -ret = pci_clean_dpci_irqs(d);
> +ret = arch_pci_clean_irqs(d);

One note regarding the naming: I'd prefer if "dpci" remained part of
the name, unless you mean to get rid of it altogether from the code
base. That'll help (a little) with visually separating code pieces
belonging to the host side of PCI from those belonging to the
passed through side of it (granted - they aren't entirely independent
anyway).

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c

2018-02-26 Thread Julien Grall 

Hi,

On 21/02/18 21:46, Wei Liu wrote:

Move the functions that reference x86 hvm data structures to its own
file.  Rename pci_clean_dpci_irqs to arch_pci_clean_irqs.


NIT: Double space.



There is still one location in that file which references
arch.hvm_domain, but it is fine because ARM guest is HVM.


I guess you mean hvm_domain.mem_sharing_enabled? If so, we don't support 
memory sharing on ARM. This would need to be stub.




Signed-off-by: Wei Liu 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Jan Beulich 
Cc: Andrew Cooper 

ARM doesn't select HAS_PCI, that's why ARM build is not broken by
this. AIUI ARM will select HAS_PCI at some point, hence I only move
the x86 bits.


Potentially there are more x86-ism in that code such as the way to 
handle MSI. Anyway, this patch makes sense from an Arm perspective:


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH RFC 02/10] passthrough: split out x86 PCI code to x86/pci.c

2018-02-21 Thread Wei Liu 
Move the functions that reference x86 hvm data structures to its own
file.  Rename pci_clean_dpci_irqs to arch_pci_clean_irqs.

There is still one location in that file which references
arch.hvm_domain, but it is fine because ARM guest is HVM.

Signed-off-by: Wei Liu 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Jan Beulich 
Cc: Andrew Cooper 

ARM doesn't select HAS_PCI, that's why ARM build is not broken by
this. AIUI ARM will select HAS_PCI at some point, hence I only move
the x86 bits.
---
 xen/drivers/passthrough/pci.c| 51 +
 xen/drivers/passthrough/x86/Makefile |  1 +
 xen/drivers/passthrough/x86/pci.c| 62 
 xen/include/xen/pci.h|  2 ++
 4 files changed, 66 insertions(+), 50 deletions(-)
 create mode 100644 xen/drivers/passthrough/x86/pci.c

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 2b976ade62..bf83d07279 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -22,7 +22,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -798,54 +797,6 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
 return ret;
 }
 
-static int pci_clean_dpci_irq(struct domain *d,
-  struct hvm_pirq_dpci *pirq_dpci, void *arg)
-{
-struct dev_intx_gsi_link *digl, *tmp;
-
-pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
-
-if ( pt_irq_need_timer(pirq_dpci->flags) )
-kill_timer(&pirq_dpci->timer);
-
-list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
-{
-list_del(&digl->list);
-xfree(digl);
-}
-
-return pt_pirq_softirq_active(pirq_dpci) ? -ERESTART : 0;
-}
-
-static int pci_clean_dpci_irqs(struct domain *d)
-{
-struct hvm_irq_dpci *hvm_irq_dpci = NULL;
-
-if ( !iommu_enabled )
-return 0;
-
-if ( !is_hvm_domain(d) )
-return 0;
-
-spin_lock(&d->event_lock);
-hvm_irq_dpci = domain_get_irq_dpci(d);
-if ( hvm_irq_dpci != NULL )
-{
-int ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
-
-if ( ret )
-{
-spin_unlock(&d->event_lock);
-return ret;
-}
-
-hvm_domain_irq(d)->dpci = NULL;
-free_hvm_irq_dpci(hvm_irq_dpci);
-}
-spin_unlock(&d->event_lock);
-return 0;
-}
-
 int pci_release_devices(struct domain *d)
 {
 struct pci_dev *pdev;
@@ -853,7 +804,7 @@ int pci_release_devices(struct domain *d)
 int ret;
 
 pcidevs_lock();
-ret = pci_clean_dpci_irqs(d);
+ret = arch_pci_clean_irqs(d);
 if ( ret )
 {
 pcidevs_unlock();
diff --git a/xen/drivers/passthrough/x86/Makefile 
b/xen/drivers/passthrough/x86/Makefile
index 06971707f8..0a21b60b5a 100644
--- a/xen/drivers/passthrough/x86/Makefile
+++ b/xen/drivers/passthrough/x86/Makefile
@@ -4,3 +4,4 @@ subdir-y += amd
 obj-y += ats.o
 obj-y += io.o
 obj-y += iommu.o
+obj-y += pci.o
diff --git a/xen/drivers/passthrough/x86/pci.c 
b/xen/drivers/passthrough/x86/pci.c
new file mode 100644
index 00..e0a7e473b1
--- /dev/null
+++ b/xen/drivers/passthrough/x86/pci.c
@@ -0,0 +1,62 @@
+#include 
+#include 
+#include 
+
+#include 
+
+static int pci_clean_dpci_irq(struct domain *d,
+  struct hvm_pirq_dpci *pirq_dpci, void *arg)
+{
+struct dev_intx_gsi_link *digl, *tmp;
+
+pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
+
+if ( pt_irq_need_timer(pirq_dpci->flags) )
+kill_timer(&pirq_dpci->timer);
+
+list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
+{
+list_del(&digl->list);
+xfree(digl);
+}
+
+return pt_pirq_softirq_active(pirq_dpci) ? -ERESTART : 0;
+}
+
+int arch_pci_clean_irqs(struct domain *d)
+{
+struct hvm_irq_dpci *hvm_irq_dpci = NULL;
+
+if ( !iommu_enabled )
+return 0;
+
+if ( !is_hvm_domain(d) )
+return 0;
+
+spin_lock(&d->event_lock);
+hvm_irq_dpci = domain_get_irq_dpci(d);
+if ( hvm_irq_dpci != NULL )
+{
+int ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
+
+if ( ret )
+{
+spin_unlock(&d->event_lock);
+return ret;
+}
+
+hvm_domain_irq(d)->dpci = NULL;
+free_hvm_irq_dpci(hvm_irq_dpci);
+}
+spin_unlock(&d->event_lock);
+return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index dd5ec43a70..2d3bdf386f 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -153,6 +153,8 @@ struct pci_dev *pci_get_pdev_by_domain(const struct domain 
*, int seg,
int bus, int devfn);
 void pci_check_disable_device(u16 seg, u8 bus, u8 devfn);
 
+int arch_pci_clean_irqs(struct domain *d);
+
 uint8_t pci_conf_read8(
 unsigned int seg, unsigned int bus, u