[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN v2] GICv3: Emulate GICD_IGRPMODR as RAZ / WI


  • To: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • From: Ayan Kumar Halder <ayankuma@xxxxxxx>
  • Date: Mon, 24 Oct 2022 19:28:38 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ipa5zbm/riYkk9XDmnfmhMaJUxeYgVAjYPpTlvYMJho=; b=Ruchb+rBlrpZeVC1fw5vqTBCeKZ6E4qmsgVcbdN/3RhRhqQMVYyhfR+8FXRLcwTZod/uLm0yhaQH4xW2clU7cOW2G5Po+k/2qMr8bUyyOS9VdaOJYFvDppwS96X198EgtfAyspbWQK35bO9/nt7qxP/T641kt9w5v459toaMphqPfsxyn33z/Opm6nEGMvlrtO90TmWYVg9NbkZyJnkk2kFnqziXkb3dTdiIpU8Y8/5Bhia8TtVmzNMEbX2RQAE49CRUrjLJSeYV+mMm2Y4lh1hTaOxDfCRAW2+E8G0J2QM+Ohd4dVjHDRzaTr34eRXyqkw7EBWIvRfwTgelDM9jRQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VPNePxQA4cOTUo728MT7/hQl5lVXaD6BNj3yhlusRDe5xoIUCGcHC6qGNuBA3WQ9UrzID2004YdkkxguTfGNh5ApslMXgt4+j3VpTdqNzJdwECe1FyFs5s4SrH54AhYA8JAL4dLelBcJmpL3TfVeXVCMmczDgXqmmuTPDx5btARhmaNs+DpRnDDGHOfBm1Qx2PMiFJ+pUlU/P3nl3ewbCLytPuRZ2bLXx7fuNKHGkMyfe9SaP0m98VbKgIS3F+OMYaBqHUy2QlejStvr9HnUlrj1Njnps36ma4KsUI3Rgq+UtD4+8FRVTLXJPssC3M2nhYB1+fKL77YHUOi9RKjAyw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, "stefanos@xxxxxxxxxx" <stefanos@xxxxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, "Volodymyr_Babchuk@xxxxxxxx" <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Mon, 24 Oct 2022 18:28:59 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>


On 24/10/2022 14:22, Bertrand Marquis wrote:
Hi Ayan,
Hi Bertrand,

On 24 Oct 2022, at 14:11, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:


On 24/10/2022 11:06, Bertrand Marquis wrote:
Hi Ayan,
Hi Bertrand,
On 20 Oct 2022, at 11:41, Ayan Kumar Halder <ayankuma@xxxxxxx> wrote:

Refer GIC v3 specification (Arm IHI 0069H ID020922), IGRPMODR (similar to
IGROUPR) is relevant only when the guests run in secure/non-secure mode.
This sentence is a bit misleading as guests are always running in either secure 
or non-secure.
Oh, my understanding from the comment "We do not implement security extensions for 
guests" is that Xen does not allow guests to run in secure mode.

Also, does Xen itself ever run in secure mode ? I thought it was no.

 From https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions

"The primary requirement is that the hypervisor must be launched in Non-Secure 
Hypervisor mode only."
For a long time there was no EL2 in secure mode so that was not even possible.
This has been introduced in the past year but nobody ever tested that apart 
from the work on R82 and R52.

So for now, Xen must be launched in non secure mode, any other setup is 
unsupported (might work though).

We should just say that we do not want guest to change the group of interrupts 
so we do as if all guests are running in non-secure.

As Xen does not implement security extensions for guests, so the registers
are emulated as read as zero/write ignore.
I would rephrase this as “Xen does support to run in secure mode so emulate all 
registers as the hardware does in non-secure.”
Do you mean ?

" Xen does *not* support *guests* to run in secure mode so emulate all registers as 
the hardware does in non-secure."
A guest is always running in the same mode as Xen.

There is a question for guest running in secure mode when (if) Xen will run in 
secure mode: what rights can we give to guest ?
 From the theory point of view, it does not make sense for a guest to play with 
the groups I think, as interrupt management is to be done by Xen.

So I think it makes sense to say that those hardware registers are not 
accessible to Xen guests as Xen will have to be the one programming interrupt 
to be fired in secure or non secure world.

Many thanks for the explanation. It makes sense.

I have sent out "[XEN v3 01/13] GICv3: Emulate GICD_IGRPMODR as RAZ / WI" with the updated commit message.

- Ayan


Cheers
Bertrand

- Ayan

On a side note, the question might come at some point if we support to run from 
secure mode on hardware supporting it, it could be that dom0 or Xen itself 
would need to modify those.

The code is ok, just the commit message would need a bit of rework I think.

Cheers
Bertrand

Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxx>
---

Observed the issue while running Zephyr on R52.
Also, found that KVM has similar behaviour.

Changes from:-
v1 - Moved the definitions of GICD_IGRPMODR, GICD_IGRPMODRN to gic_v3
specific header.

xen/arch/arm/include/asm/gic_v3_defs.h | 2 ++
xen/arch/arm/vgic-v3.c                 | 4 ++++
2 files changed, 6 insertions(+)

diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h 
b/xen/arch/arm/include/asm/gic_v3_defs.h
index 34ed5f857d..728e28d5e5 100644
--- a/xen/arch/arm/include/asm/gic_v3_defs.h
+++ b/xen/arch/arm/include/asm/gic_v3_defs.h
@@ -30,6 +30,8 @@
#define GICD_CLRSPI_NSR              (0x048)
#define GICD_SETSPI_SR               (0x050)
#define GICD_CLRSPI_SR               (0x058)
+#define GICD_IGRPMODR                (0xD00)
+#define GICD_IGRPMODRN               (0xD7C)
#define GICD_IROUTER                 (0x6000)
#define GICD_IROUTER32               (0x6100)
#define GICD_IROUTER1019             (0x7FD8)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 7fb99a9ff2..0c23f6df9d 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -685,6 +685,7 @@ static int __vgic_v3_distr_common_mmio_read(const char 
*name, struct vcpu *v,
     switch ( reg )
     {
     case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
         /* We do not implement security extensions for guests, read zero */
         if ( dabt.size != DABT_WORD ) goto bad_width;
         goto read_as_zero;
@@ -781,6 +782,7 @@ static int __vgic_v3_distr_common_mmio_write(const char 
*name, struct vcpu *v,
     switch ( reg )
     {
     case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
         /* We do not implement security extensions for guests, write ignore */
         goto write_ignore_32;

@@ -1192,6 +1194,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, 
mmio_info_t *info,
     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
     case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
     case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
         /*
          * Above all register are common with GICR and GICD
          * Manage in common
@@ -1379,6 +1382,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info,
     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
     case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
     case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
         /* Above registers are common with GICR and GICD
          * Manage in common */
         return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
--
2.17.1




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.