[Xen-devel,v3,01/62] Revert "xen/arm: vgic-v2: Drop cbase from arch_domain"

Message ID 1447753261-7552-2-git-send-email-shannon.zhao@linaro.org
State New
Headers show

Commit Message

Shannon Zhao Nov. 17, 2015, 9:40 a.m.
From: Shannon Zhao <shannon.zhao@linaro.org>


This reverts commit 810a50db69703f715d199d6b3a5f08193155d48b.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>


Conflicts:
	xen/arch/arm/vgic-v2.c
---
 xen/arch/arm/vgic-v2.c       | 10 +++++-----
 xen/include/asm-arm/domain.h |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.1.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Comments

Shannon Zhao Nov. 18, 2015, 1:09 p.m. | #1
On 2015/11/18 19:41, Julien Grall wrote:
> Hi Shannon,

>

> On 18/11/15 02:28, Shannon Zhao wrote:

>

>> On 2015/11/17 21:57, Julien Grall wrote:

>>> On 17/11/15 12:32, Shannon Zhao wrote:

>>>>> Hi Julien,

>>>>>

>>>>> On 2015/11/17 19:27, Julien Grall wrote:

>>>>>>> Hi Shannon,

>>>>>>>

>>>>>>> Why do you want to revert this patch?

>>>>>>>

>>>>> Because d->arch.vgic.cbase will be used by creating Dom0 MADT table

>>>>> later. See [PATCH v3 43/62].

>>>>> +            gicc.base_address = d->arch.vgic.cbase;

>>>>>

>>>>> My previous way is get this from ACPI table but someone suggest get it

>>>>> from struct domain and I think this way is better too since it uses the

>>>>> value after being parsed.

>>> It's pointless to store the value in arch_domain for something that it's

>>> only use during building...

>>>

>>> We have struct kernel_info which store any information related to the

>>> DOM0 during the building.

>>>

>>

>> Yeah, that would be better if it could use kernel_info. But the problem

>> is that kernel_info is firstly used in construct_dom0(), while it needs

>> to store the cbase in domain_create() which is called before

>> construct_dom0(). And if we pass kernel_info as parameter to

>> domain_create(), this would introduce more changes to common codes.

>>

>> Do you have any better idea to handle this?

>

> Yes, introducing callback to create the ACPI table in the GIC driver.

> See what we did for make_hwdom_dt_node.

>

> We are trying to make the domain_build domain as agnostic as possible

> from the GIC version. However in patch #43, you implement specific

> version in the domain builder.

>

> It will also not scale very well when we will introduce GICv2m and ITS.

>

> The version of the GIC (ACPI_MADT_GIC_*) could be introduced in the

> vgic_ops.

>

> The re-distributor could be moved in gic-v3.c and the generic interrupt

> controller in gic-v2.c.

>

> Note it's a mandatory to emulate the same version as the hardware for

> the virtual GIC.

>

So it doesn't support vGICv2 on GICv3 hardware for Xen?

Thanks,
-- 
Shannon

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Shannon Zhao Nov. 18, 2015, 1:38 p.m. | #2
On 2015/11/18 21:33, Julien Grall wrote:
> On 18/11/15 13:09, Shannon Zhao wrote:

>>> Note it's a mandatory to emulate the same version as the hardware for

>>> the virtual GIC.

>>>

>> So it doesn't support vGICv2 on GICv3 hardware for Xen?

>

> Sorry I forgot to say it was for DOM0. We support GICv2 on GICv3 only

> for guest.

>

Ok, I see. Thanks for your explanation.

-- 
Shannon

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Patch

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index f7d784b..ba7ddac 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -531,7 +531,7 @@  static int vgic_v2_vcpu_init(struct vcpu *v)
 static int vgic_v2_domain_init(struct domain *d)
 {
     int i, ret;
-    paddr_t cbase, csize;
+    paddr_t csize;
     paddr_t vbase;
 
     /*
@@ -541,6 +541,7 @@  static int vgic_v2_domain_init(struct domain *d)
     if ( is_hardware_domain(d) )
     {
         d->arch.vgic.dbase = vgic_v2_hw.dbase;
+        d->arch.vgic.cbase = vgic_v2_hw.cbase;
         /*
          * For the hardware domain, we always map the whole HW CPU
          * interface region in order to match the device tree (the "reg"
@@ -548,13 +549,13 @@  static int vgic_v2_domain_init(struct domain *d)
          * Note that we assume the size of the CPU interface is always
          * aligned to PAGE_SIZE.
          */
-        cbase = vgic_v2_hw.cbase;
         csize = vgic_v2_hw.csize;
         vbase = vgic_v2_hw.vbase;
     }
     else
     {
         d->arch.vgic.dbase = GUEST_GICD_BASE;
+        d->arch.vgic.cbase = GUEST_GICC_BASE;
         /*
          * The CPU interface exposed to the guest is always 8kB. We may
          * need to add an offset to the virtual CPU interface base
@@ -562,7 +563,6 @@  static int vgic_v2_domain_init(struct domain *d)
          * region.
          */
         BUILD_BUG_ON(GUEST_GICC_SIZE != SZ_8K);
-        cbase = GUEST_GICC_BASE;
         csize = GUEST_GICC_SIZE;
         vbase = vgic_v2_hw.vbase + vgic_v2_hw.aliased_offset;
     }
@@ -571,8 +571,8 @@  static int vgic_v2_domain_init(struct domain *d)
      * Map the gic virtual cpu interface in the gic cpu interface
      * region of the guest.
      */
-    ret = map_mmio_regions(d, paddr_to_pfn(cbase), csize / PAGE_SIZE,
-                           paddr_to_pfn(vbase));
+    ret = map_mmio_regions(d, paddr_to_pfn(d->arch.vgic.cbase),
+                           csize / PAGE_SIZE, paddr_to_pfn(vbase));
     if ( ret )
         return ret;
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index e7e40da..1e61f30 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -102,6 +102,7 @@  struct arch_domain
         struct pending_irq *pending_irqs;
         /* Base address for guest GIC */
         paddr_t dbase; /* Distributor base address */
+        paddr_t cbase; /* CPU base address */
 #ifdef HAS_GICV3
         /* GIC V3 addressing */
         /* List of contiguous occupied by the redistributors */