diff mbox series

[v4,23/33] sgx-epc: Add the reset interface for sgx-epc virt device

Message ID 20210719112136.57018-24-yang.zhong@intel.com
State New
Headers show
Series Qemu SGX virtualization | expand

Commit Message

Yang Zhong July 19, 2021, 11:21 a.m. UTC 
If the VM is reset, we need make sure sgx virt epc in clean status.
Once the VM is reset, and sgx epc virt device will be reseted by
reset callback registered by qemu_register_reset(). Since this epc
virt device depend on backend, this reset will call backend reset
interface to re-mmap epc to guest.

Signed-off-by: Yang Zhong <yang.zhong@intel.com>
---
 hw/i386/sgx-epc.c | 94 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 81 insertions(+), 13 deletions(-)

Comments

Paolo Bonzini Sept. 10, 2021, 3:13 p.m. UTC | #1 
On 19/07/21 13:21, Yang Zhong wrote:
> +static void sgx_epc_del_subregion(DeviceState *dev)
> +{
> +    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> +    SGXEPCState *sgx_epc = &pcms->sgx_epc;
> +    SGXEPCDevice *epc = SGX_EPC(dev);
> +
> +    /* del subregion and related operations */
> +    memory_region_del_subregion(&sgx_epc->mr,
> +                                host_memory_backend_get_memory(epc->hostmem));
> +    host_memory_backend_set_mapped(epc->hostmem, false);
> +    g_free(sgx_epc->sections);
> +    sgx_epc->sections = NULL;
> +
> +    /* multiple epc devices, only zero the first time */
> +    if (epc_num == sgx_epc->nr_sections) {
> +        sgx_epc->size = 0;
> +        sgx_epc->nr_sections = 0;
> +    }
> +}

Can it just invoke a method on the memory devices, without reusing the 
code in sgx_epc_realize?  In particular why is it necessary to update 
sgx_epc->size and sgx_epc->nr_sections?

Paolo
Philippe Mathieu-Daudé Sept. 14, 2021, 6:53 a.m. UTC | #2 
On 7/19/21 1:21 PM, Yang Zhong wrote:
> If the VM is reset, we need make sure sgx virt epc in clean status.
> Once the VM is reset, and sgx epc virt device will be reseted by
> reset callback registered by qemu_register_reset(). Since this epc
> virt device depend on backend, this reset will call backend reset
> interface to re-mmap epc to guest.
> 
> Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> ---
>  hw/i386/sgx-epc.c | 94 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 81 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/i386/sgx-epc.c b/hw/i386/sgx-epc.c
> index 924dea22f0..9880d832d5 100644
> --- a/hw/i386/sgx-epc.c
> +++ b/hw/i386/sgx-epc.c
> @@ -18,6 +18,9 @@
>  #include "qapi/visitor.h"
>  #include "target/i386/cpu.h"
>  #include "exec/address-spaces.h"
> +#include "sysemu/reset.h"
> +
> +uint32_t epc_num;

Missing 'static' qualifier.
Yang Zhong Sept. 15, 2021, 11:33 a.m. UTC | #3 
On Tue, Sep 14, 2021 at 08:53:48AM +0200, Philippe Mathieu-Daudé wrote:
> On 7/19/21 1:21 PM, Yang Zhong wrote:
> > If the VM is reset, we need make sure sgx virt epc in clean status.
> > Once the VM is reset, and sgx epc virt device will be reseted by
> > reset callback registered by qemu_register_reset(). Since this epc
> > virt device depend on backend, this reset will call backend reset
> > interface to re-mmap epc to guest.
> > 
> > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> > ---
> >  hw/i386/sgx-epc.c | 94 ++++++++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 81 insertions(+), 13 deletions(-)
> > 
> > diff --git a/hw/i386/sgx-epc.c b/hw/i386/sgx-epc.c
> > index 924dea22f0..9880d832d5 100644
> > --- a/hw/i386/sgx-epc.c
> > +++ b/hw/i386/sgx-epc.c
> > @@ -18,6 +18,9 @@
> >  #include "qapi/visitor.h"
> >  #include "target/i386/cpu.h"
> >  #include "exec/address-spaces.h"
> > +#include "sysemu/reset.h"
> > +
> > +uint32_t epc_num;
> 
> Missing 'static' qualifier.

  Philippe, thanks! This Qemu SGX reset solution will be replaced by kernel
  patchset made by Paolo(https://www.spinics.net/lists/kvm/msg253930.html).

  Yang
diff mbox series

Patch

diff --git a/hw/i386/sgx-epc.c b/hw/i386/sgx-epc.c
index 924dea22f0..9880d832d5 100644
--- a/hw/i386/sgx-epc.c
+++ b/hw/i386/sgx-epc.c
@@ -18,6 +18,9 @@ 
 #include "qapi/visitor.h"
 #include "target/i386/cpu.h"
 #include "exec/address-spaces.h"
+#include "sysemu/reset.h"
+
+uint32_t epc_num;
 
 static Property sgx_epc_properties[] = {
     DEFINE_PROP_UINT64(SGX_EPC_ADDR_PROP, SGXEPCDevice, addr, 0),
@@ -47,12 +50,84 @@  static void sgx_epc_init(Object *obj)
                         NULL, NULL, NULL);
 }
 
+static void sgx_epc_del_subregion(DeviceState *dev)
+{
+    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+    SGXEPCState *sgx_epc = &pcms->sgx_epc;
+    SGXEPCDevice *epc = SGX_EPC(dev);
+
+    /* del subregion and related operations */
+    memory_region_del_subregion(&sgx_epc->mr,
+                                host_memory_backend_get_memory(epc->hostmem));
+    host_memory_backend_set_mapped(epc->hostmem, false);
+    g_free(sgx_epc->sections);
+    sgx_epc->sections = NULL;
+
+    /* multiple epc devices, only zero the first time */
+    if (epc_num == sgx_epc->nr_sections) {
+        sgx_epc->size = 0;
+        sgx_epc->nr_sections = 0;
+    }
+}
+
+static void sgx_epc_initialization(DeviceState *dev)
+{
+    PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+    SGXEPCState *sgx_epc = &pcms->sgx_epc;
+    MemoryDeviceState *md = MEMORY_DEVICE(dev);
+    SGXEPCDevice *epc = SGX_EPC(dev);
+    Error *errp = NULL;
+
+    if (!epc->hostmem) {
+        error_setg(&errp, "'" SGX_EPC_MEMDEV_PROP "' property is not set");
+        return;
+    }
+
+    epc->addr = sgx_epc->base + sgx_epc->size;
+
+    memory_region_add_subregion(&sgx_epc->mr, epc->addr - sgx_epc->base,
+                                host_memory_backend_get_memory(epc->hostmem));
+
+    host_memory_backend_set_mapped(epc->hostmem, true);
+
+    sgx_epc->sections = g_renew(SGXEPCDevice *, sgx_epc->sections,
+                                sgx_epc->nr_sections + 1);
+    sgx_epc->sections[sgx_epc->nr_sections++] = epc;
+
+    sgx_epc->size += memory_device_get_region_size(md, &errp);
+}
+
+static void sgx_epc_reset(void *opaque)
+{
+    DeviceState *dev = opaque;
+    SGXEPCDevice *epc = SGX_EPC(dev);
+    Error *errp = NULL;
+    int fd;
+
+    if (!epc->hostmem) {
+        error_setg(&errp, "'" SGX_EPC_MEMDEV_PROP "' property is not set");
+        return;
+    }
+
+    /* delete subregion and related operations */
+    sgx_epc_del_subregion(dev);
+
+    /* reset sgx backend */
+    fd = memory_region_get_fd(host_memory_backend_get_memory(epc->hostmem));
+    sgx_memory_backend_reset(epc->hostmem, fd, &errp);
+    if (errp) {
+        error_setg(&errp, "failed to call sgx_memory_backend_reset");
+        return;
+    }
+
+    /* re-add subregion and related operations */
+    sgx_epc_initialization(dev);
+}
+
 static void sgx_epc_realize(DeviceState *dev, Error **errp)
 {
     PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
     X86MachineState *x86ms = X86_MACHINE(pcms);
-    MemoryDeviceState *md = MEMORY_DEVICE(dev);
-    SGXEPCState *sgx_epc = &pcms->sgx_epc;
     SGXEPCDevice *epc = SGX_EPC(dev);
     const char *path;
 
@@ -71,18 +146,11 @@  static void sgx_epc_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    epc->addr = sgx_epc->base + sgx_epc->size;
-
-    memory_region_add_subregion(&sgx_epc->mr, epc->addr - sgx_epc->base,
-                                host_memory_backend_get_memory(epc->hostmem));
-
-    host_memory_backend_set_mapped(epc->hostmem, true);
-
-    sgx_epc->sections = g_renew(SGXEPCDevice *, sgx_epc->sections,
-                                sgx_epc->nr_sections + 1);
-    sgx_epc->sections[sgx_epc->nr_sections++] = epc;
+    sgx_epc_initialization(dev);
+    epc_num++;
 
-    sgx_epc->size += memory_device_get_region_size(md, errp);
+    /* register the reset callback for sgx reset */
+    qemu_register_reset(sgx_epc_reset, dev);
 }
 
 static void sgx_epc_unrealize(DeviceState *dev)