Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
On 7/2/2020 9:42 PM, Auger Eric wrote:
Hi Marc,
On 7/2/20 3:08 PM, Marc Zyngier wrote:
Hi Eric,
On 2020-07-02 13:57, Auger Eric wrote:
Hi Jingyi,
On 7/2/20 5:01 AM, Jingyi Wang wrote:
If gicv4.1(sgi hardware injection) supported, we test ipi injection
via hw/sw way separately.
Signed-off-by: Jingyi Wang
---
arm/micro-bench.c | 45 +++-
lib/arm/asm/gic-v3.h | 3 +++
lib/arm/asm/gic.h | 1 +
3 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index fc4d356..80d8db3 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -91,9 +91,40 @@ static void gic_prep_common(void)
assert(irq_ready);
}
-static void ipi_prep(void)
+static bool ipi_prep(void)
Any reason why the bool returned value is preferred over the standard
int?
{
+ u32 val;
+
+ val = readl(vgic_dist_base + GICD_CTLR);
+ if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
+ val &= ~GICD_CTLR_ENABLE_G1A;
+ val &= ~GICD_CTLR_nASSGIreq;
+ writel(val, vgic_dist_base + GICD_CTLR);
+ val |= GICD_CTLR_ENABLE_G1A;
+ writel(val, vgic_dist_base + GICD_CTLR);
Why do we need this G1A dance?
Because it isn't legal to change the SGI behaviour when groups are enabled.
Yes, it is described in this bit of documentation nobody has access to.
OK thank you for the explanation. Jingyi, maybe add a comment to avoid
the question again ;-)
Thanks
Eric
Okay, I will add a comment here in the next version.
Thanks,
Jingyi
And this code needs to track RWP on disabling Group-1.
M.
.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
On Thu, Jul 02, 2020 at 02:57:56PM +0200, Auger Eric wrote: > Hi Jingyi, > > On 7/2/20 5:01 AM, Jingyi Wang wrote: > > If gicv4.1(sgi hardware injection) supported, we test ipi injection > > via hw/sw way separately. > > > > Signed-off-by: Jingyi Wang > > --- > > arm/micro-bench.c| 45 +++- > > lib/arm/asm/gic-v3.h | 3 +++ > > lib/arm/asm/gic.h| 1 + > > 3 files changed, 44 insertions(+), 5 deletions(-) > > > > diff --git a/arm/micro-bench.c b/arm/micro-bench.c > > index fc4d356..80d8db3 100644 > > --- a/arm/micro-bench.c > > +++ b/arm/micro-bench.c > > @@ -91,9 +91,40 @@ static void gic_prep_common(void) > > assert(irq_ready); > > } > > > > -static void ipi_prep(void) > > +static bool ipi_prep(void) > Any reason why the bool returned value is preferred over the standard int? Why would an int be preferred over bool if the return is boolean? Thanks, drew ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
Hi Marc,
On 7/2/20 3:08 PM, Marc Zyngier wrote:
> Hi Eric,
>
> On 2020-07-02 13:57, Auger Eric wrote:
>> Hi Jingyi,
>>
>> On 7/2/20 5:01 AM, Jingyi Wang wrote:
>>> If gicv4.1(sgi hardware injection) supported, we test ipi injection
>>> via hw/sw way separately.
>>>
>>> Signed-off-by: Jingyi Wang
>>> ---
>>> arm/micro-bench.c | 45 +++-
>>> lib/arm/asm/gic-v3.h | 3 +++
>>> lib/arm/asm/gic.h | 1 +
>>> 3 files changed, 44 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
>>> index fc4d356..80d8db3 100644
>>> --- a/arm/micro-bench.c
>>> +++ b/arm/micro-bench.c
>>> @@ -91,9 +91,40 @@ static void gic_prep_common(void)
>>> assert(irq_ready);
>>> }
>>>
>>> -static void ipi_prep(void)
>>> +static bool ipi_prep(void)
>> Any reason why the bool returned value is preferred over the standard
>> int?
>>> {
>>> + u32 val;
>>> +
>>> + val = readl(vgic_dist_base + GICD_CTLR);
>>> + if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
>>> + val &= ~GICD_CTLR_ENABLE_G1A;
>>> + val &= ~GICD_CTLR_nASSGIreq;
>>> + writel(val, vgic_dist_base + GICD_CTLR);
>>> + val |= GICD_CTLR_ENABLE_G1A;
>>> + writel(val, vgic_dist_base + GICD_CTLR);
>> Why do we need this G1A dance?
>
> Because it isn't legal to change the SGI behaviour when groups are enabled.
> Yes, it is described in this bit of documentation nobody has access to.
OK thank you for the explanation. Jingyi, maybe add a comment to avoid
the question again ;-)
Thanks
Eric
>
> And this code needs to track RWP on disabling Group-1.
>
> M.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
Hi Eric,
On 2020-07-02 13:57, Auger Eric wrote:
Hi Jingyi,
On 7/2/20 5:01 AM, Jingyi Wang wrote:
If gicv4.1(sgi hardware injection) supported, we test ipi injection
via hw/sw way separately.
Signed-off-by: Jingyi Wang
---
arm/micro-bench.c| 45
+++-
lib/arm/asm/gic-v3.h | 3 +++
lib/arm/asm/gic.h| 1 +
3 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index fc4d356..80d8db3 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -91,9 +91,40 @@ static void gic_prep_common(void)
assert(irq_ready);
}
-static void ipi_prep(void)
+static bool ipi_prep(void)
Any reason why the bool returned value is preferred over the standard
int?
{
+ u32 val;
+
+ val = readl(vgic_dist_base + GICD_CTLR);
+ if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
+ val &= ~GICD_CTLR_ENABLE_G1A;
+ val &= ~GICD_CTLR_nASSGIreq;
+ writel(val, vgic_dist_base + GICD_CTLR);
+ val |= GICD_CTLR_ENABLE_G1A;
+ writel(val, vgic_dist_base + GICD_CTLR);
Why do we need this G1A dance?
Because it isn't legal to change the SGI behaviour when groups are
enabled.
Yes, it is described in this bit of documentation nobody has access to.
And this code needs to track RWP on disabling Group-1.
M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
Hi Eric, On 2020-07-02 13:36, Auger Eric wrote: Hi Marc, On 7/2/20 10:22 AM, Marc Zyngier wrote: On 2020-07-02 04:01, Jingyi Wang wrote: If gicv4.1(sgi hardware injection) supported, we test ipi injection via hw/sw way separately. nit: active-less SGIs are not strictly a feature of GICv4.1 (you could imagine a GIC emulation offering the same thing). Furthermore, GICv4.1 isn't as such visible to the guest itself (it only sees a GICv3). By the way, I have just downloaded the latest GIC spec from the ARM portal and I still do not find the GICD_CTLR_ENABLE_G1A, GICD_CTLR_nASSGIreq and GICD_TYPER2_nASSGIcap. Do I miss something? The latest spec still is the old one. There is a *confidential* erratum to the spec that adds the missing bits, but nothing public. You unfortunately will have to take my word for it. M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
Hi Jingyi,
On 7/2/20 5:01 AM, Jingyi Wang wrote:
> If gicv4.1(sgi hardware injection) supported, we test ipi injection
> via hw/sw way separately.
>
> Signed-off-by: Jingyi Wang
> ---
> arm/micro-bench.c| 45 +++-
> lib/arm/asm/gic-v3.h | 3 +++
> lib/arm/asm/gic.h| 1 +
> 3 files changed, 44 insertions(+), 5 deletions(-)
>
> diff --git a/arm/micro-bench.c b/arm/micro-bench.c
> index fc4d356..80d8db3 100644
> --- a/arm/micro-bench.c
> +++ b/arm/micro-bench.c
> @@ -91,9 +91,40 @@ static void gic_prep_common(void)
> assert(irq_ready);
> }
>
> -static void ipi_prep(void)
> +static bool ipi_prep(void)
Any reason why the bool returned value is preferred over the standard int?
> {
> + u32 val;
> +
> + val = readl(vgic_dist_base + GICD_CTLR);
> + if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
> + val &= ~GICD_CTLR_ENABLE_G1A;
> + val &= ~GICD_CTLR_nASSGIreq;
> + writel(val, vgic_dist_base + GICD_CTLR);
> + val |= GICD_CTLR_ENABLE_G1A;
> + writel(val, vgic_dist_base + GICD_CTLR);
Why do we need this G1A dance?
> + }
> +
> gic_prep_common();
> + return true;
> +}
> +
> +static bool ipi_hw_prep(void)
> +{
> + u32 val;
> +
> + val = readl(vgic_dist_base + GICD_CTLR);
> + if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
> + val &= ~GICD_CTLR_ENABLE_G1A;
> + val |= GICD_CTLR_nASSGIreq;
> + writel(val, vgic_dist_base + GICD_CTLR);
> + val |= GICD_CTLR_ENABLE_G1A;
> + writel(val, vgic_dist_base + GICD_CTLR);
> + } else {
> + return false;
> + }
> +
> + gic_prep_common();
> + return true;
> }
>
> static void ipi_exec(void)
> @@ -147,7 +178,7 @@ static void eoi_exec(void)
>
> struct exit_test {
> const char *name;
> - void (*prep)(void);
> + bool (*prep)(void);
> void (*exec)(void);
> bool run;
> };
> @@ -158,6 +189,7 @@ static struct exit_test tests[] = {
> {"mmio_read_vgic", NULL, mmio_read_vgic_exec,true},
> {"eoi", NULL, eoi_exec, true},
> {"ipi", ipi_prep, ipi_exec, true},
> + {"ipi_hw", ipi_hw_prep,ipi_exec, true},
> };
>
> struct ns_time {
> @@ -181,9 +213,12 @@ static void loop_test(struct exit_test *test)
> uint64_t start, end, total_ticks, ntimes = NTIMES;
> struct ns_time total_ns, avg_ns;
>
> - if (test->prep)
> - test->prep();
> -
> + if (test->prep) {
> + if(!test->prep()) {
> + printf("%s test skipped\n", test->name);
> + return;
> + }
> + }
> isb();
> start = read_sysreg(cntpct_el0);
> while (ntimes--)
> diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
> index cb72922..b4ce130 100644
> --- a/lib/arm/asm/gic-v3.h
> +++ b/lib/arm/asm/gic-v3.h
> @@ -20,10 +20,13 @@
> */
> #define GICD_CTLR0x
> #define GICD_CTLR_RWP(1U << 31)
> +#define GICD_CTLR_nASSGIreq (1U << 8)
> #define GICD_CTLR_ARE_NS (1U << 4)
> #define GICD_CTLR_ENABLE_G1A (1U << 1)
> #define GICD_CTLR_ENABLE_G1 (1U << 0)
>
> +#define GICD_TYPER2_nASSGIcap(1U << 8)
> +
> /* Re-Distributor registers, offsets from RD_base */
> #define GICR_TYPER 0x0008
>
> diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
> index 38e79b2..1898400 100644
> --- a/lib/arm/asm/gic.h
> +++ b/lib/arm/asm/gic.h
> @@ -13,6 +13,7 @@
> #define GICD_CTLR0x
> #define GICD_TYPER 0x0004
> #define GICD_IIDR0x0008
> +#define GICD_TYPER2 0x000C
> #define GICD_IGROUPR 0x0080
> #define GICD_ISENABLER 0x0100
> #define GICD_ICENABLER 0x0180
>
Thanks
Eric
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
Hi Marc, On 7/2/20 10:22 AM, Marc Zyngier wrote: > On 2020-07-02 04:01, Jingyi Wang wrote: >> If gicv4.1(sgi hardware injection) supported, we test ipi injection >> via hw/sw way separately. > > nit: active-less SGIs are not strictly a feature of GICv4.1 (you could > imagine a GIC emulation offering the same thing). Furthermore, GICv4.1 > isn't as such visible to the guest itself (it only sees a GICv3). By the way, I have just downloaded the latest GIC spec from the ARM portal and I still do not find the GICD_CTLR_ENABLE_G1A, GICD_CTLR_nASSGIreq and GICD_TYPER2_nASSGIcap. Do I miss something? Thanks Eric > > Thanks, > > M. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
On 7/2/2020 5:17 PM, Marc Zyngier wrote: On 2020-07-02 10:02, Jingyi Wang wrote: Hi Marc, On 7/2/2020 4:22 PM, Marc Zyngier wrote: On 2020-07-02 04:01, Jingyi Wang wrote: If gicv4.1(sgi hardware injection) supported, we test ipi injection via hw/sw way separately. nit: active-less SGIs are not strictly a feature of GICv4.1 (you could imagine a GIC emulation offering the same thing). Furthermore, GICv4.1 isn't as such visible to the guest itself (it only sees a GICv3). Thanks, M. Yes, but to measure the performance difference of hw/sw SGI injection, do you think it is acceptable to make it visible to guest and let it switch SGI injection mode? It is of course acceptable. I simply object to the "GICv4.1" description. M. Okay, maybe description like "GICv4.1 supported kvm" is better. Thanks, Jingyi ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
On 2020-07-02 10:02, Jingyi Wang wrote: Hi Marc, On 7/2/2020 4:22 PM, Marc Zyngier wrote: On 2020-07-02 04:01, Jingyi Wang wrote: If gicv4.1(sgi hardware injection) supported, we test ipi injection via hw/sw way separately. nit: active-less SGIs are not strictly a feature of GICv4.1 (you could imagine a GIC emulation offering the same thing). Furthermore, GICv4.1 isn't as such visible to the guest itself (it only sees a GICv3). Thanks, M. Yes, but to measure the performance difference of hw/sw SGI injection, do you think it is acceptable to make it visible to guest and let it switch SGI injection mode? It is of course acceptable. I simply object to the "GICv4.1" description. M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
Hi Marc, On 7/2/2020 4:22 PM, Marc Zyngier wrote: On 2020-07-02 04:01, Jingyi Wang wrote: If gicv4.1(sgi hardware injection) supported, we test ipi injection via hw/sw way separately. nit: active-less SGIs are not strictly a feature of GICv4.1 (you could imagine a GIC emulation offering the same thing). Furthermore, GICv4.1 isn't as such visible to the guest itself (it only sees a GICv3). Thanks, M. Yes, but to measure the performance difference of hw/sw SGI injection, do you think it is acceptable to make it visible to guest and let it switch SGI injection mode? Thanks, Jingyi ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
On 2020-07-02 04:01, Jingyi Wang wrote: If gicv4.1(sgi hardware injection) supported, we test ipi injection via hw/sw way separately. nit: active-less SGIs are not strictly a feature of GICv4.1 (you could imagine a GIC emulation offering the same thing). Furthermore, GICv4.1 isn't as such visible to the guest itself (it only sees a GICv3). Thanks, M. -- Jazz is not dead. It just smells funny... ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[kvm-unit-tests PATCH v2 3/8] arm64: microbench: gic: Add gicv4.1 support for ipi latency test.
If gicv4.1(sgi hardware injection) supported, we test ipi injection
via hw/sw way separately.
Signed-off-by: Jingyi Wang
---
arm/micro-bench.c| 45 +++-
lib/arm/asm/gic-v3.h | 3 +++
lib/arm/asm/gic.h| 1 +
3 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index fc4d356..80d8db3 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -91,9 +91,40 @@ static void gic_prep_common(void)
assert(irq_ready);
}
-static void ipi_prep(void)
+static bool ipi_prep(void)
{
+ u32 val;
+
+ val = readl(vgic_dist_base + GICD_CTLR);
+ if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
+ val &= ~GICD_CTLR_ENABLE_G1A;
+ val &= ~GICD_CTLR_nASSGIreq;
+ writel(val, vgic_dist_base + GICD_CTLR);
+ val |= GICD_CTLR_ENABLE_G1A;
+ writel(val, vgic_dist_base + GICD_CTLR);
+ }
+
gic_prep_common();
+ return true;
+}
+
+static bool ipi_hw_prep(void)
+{
+ u32 val;
+
+ val = readl(vgic_dist_base + GICD_CTLR);
+ if (readl(vgic_dist_base + GICD_TYPER2) & GICD_TYPER2_nASSGIcap) {
+ val &= ~GICD_CTLR_ENABLE_G1A;
+ val |= GICD_CTLR_nASSGIreq;
+ writel(val, vgic_dist_base + GICD_CTLR);
+ val |= GICD_CTLR_ENABLE_G1A;
+ writel(val, vgic_dist_base + GICD_CTLR);
+ } else {
+ return false;
+ }
+
+ gic_prep_common();
+ return true;
}
static void ipi_exec(void)
@@ -147,7 +178,7 @@ static void eoi_exec(void)
struct exit_test {
const char *name;
- void (*prep)(void);
+ bool (*prep)(void);
void (*exec)(void);
bool run;
};
@@ -158,6 +189,7 @@ static struct exit_test tests[] = {
{"mmio_read_vgic", NULL, mmio_read_vgic_exec,true},
{"eoi", NULL, eoi_exec, true},
{"ipi", ipi_prep, ipi_exec, true},
+ {"ipi_hw", ipi_hw_prep,ipi_exec, true},
};
struct ns_time {
@@ -181,9 +213,12 @@ static void loop_test(struct exit_test *test)
uint64_t start, end, total_ticks, ntimes = NTIMES;
struct ns_time total_ns, avg_ns;
- if (test->prep)
- test->prep();
-
+ if (test->prep) {
+ if(!test->prep()) {
+ printf("%s test skipped\n", test->name);
+ return;
+ }
+ }
isb();
start = read_sysreg(cntpct_el0);
while (ntimes--)
diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
index cb72922..b4ce130 100644
--- a/lib/arm/asm/gic-v3.h
+++ b/lib/arm/asm/gic-v3.h
@@ -20,10 +20,13 @@
*/
#define GICD_CTLR 0x
#define GICD_CTLR_RWP (1U << 31)
+#define GICD_CTLR_nASSGIreq(1U << 8)
#define GICD_CTLR_ARE_NS (1U << 4)
#define GICD_CTLR_ENABLE_G1A (1U << 1)
#define GICD_CTLR_ENABLE_G1(1U << 0)
+#define GICD_TYPER2_nASSGIcap (1U << 8)
+
/* Re-Distributor registers, offsets from RD_base */
#define GICR_TYPER 0x0008
diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
index 38e79b2..1898400 100644
--- a/lib/arm/asm/gic.h
+++ b/lib/arm/asm/gic.h
@@ -13,6 +13,7 @@
#define GICD_CTLR 0x
#define GICD_TYPER 0x0004
#define GICD_IIDR 0x0008
+#define GICD_TYPER20x000C
#define GICD_IGROUPR 0x0080
#define GICD_ISENABLER 0x0100
#define GICD_ICENABLER 0x0180
--
2.19.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
