date:20200425

Re: [PATCH 2/2] vdpa: implement config interrupt in IFCVF

2020-04-25 Thread Zhu Lingshan



On 4/26/2020 11:25 AM, Jason Wang wrote:


On 2020/4/24 下午6:04, Zhu Lingshan wrote:

This commit implements config interrupt support
in IFC VF

Signed-off-by: Zhu Lingshan 
---
  drivers/vdpa/ifcvf/ifcvf_base.c |  3 +++
  drivers/vdpa/ifcvf/ifcvf_base.h |  2 ++
  drivers/vdpa/ifcvf/ifcvf_main.c | 22 +-
  3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c 
b/drivers/vdpa/ifcvf/ifcvf_base.c

index b61b06e..c825d99 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.c
+++ b/drivers/vdpa/ifcvf/ifcvf_base.c
@@ -185,6 +185,9 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 
status)

    void ifcvf_reset(struct ifcvf_hw *hw)
  {
+    hw->config_cb.callback = NULL;
+    hw->config_cb.private = NULL;
+
  ifcvf_set_status(hw, 0);
  /* flush set_status, make sure VF is stopped, reset */
  ifcvf_get_status(hw);
diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h 
b/drivers/vdpa/ifcvf/ifcvf_base.h

index e803070..76928b0 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -81,6 +81,8 @@ struct ifcvf_hw {
  void __iomem *net_cfg;
  struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
  void __iomem * const *base;
+    char config_msix_name[256];
+    struct vdpa_callback config_cb;
  };
    struct ifcvf_adapter {
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c 
b/drivers/vdpa/ifcvf/ifcvf_main.c

index 8d54dc5..f7baeca 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -18,6 +18,16 @@
  #define DRIVER_AUTHOR   "Intel Corporation"
  #define IFCVF_DRIVER_NAME   "ifcvf"
  +static irqreturn_t ifcvf_config_changed(int irq, void *arg)
+{
+    struct ifcvf_hw *vf = arg;
+
+    if (vf->config_cb.callback)
+    return vf->config_cb.callback(vf->config_cb.private);
+
+    return IRQ_HANDLED;



So it looks to me the current support of VIRTIO_NET_F_STATUS is broken 
without this patch.


We probably need to patch to disable it.

Thanks


Hi Jason, We don't have VIRTIO_NET_F_STATUS in driver feature bits for 
now, you just reminded me I should add it along with this series. 
Thanks, BR Zhu Lingshan






+}
+
  static irqreturn_t ifcvf_intr_handler(int irq, void *arg)
  {
  struct vring_info *vring = arg;
@@ -256,7 +266,10 @@ static void ifcvf_vdpa_set_config(struct 
vdpa_device *vdpa_dev,

  static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
   struct vdpa_callback *cb)
  {
-    /* We don't support config interrupt */
+    struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
+
+    vf->config_cb.callback = cb->callback;
+    vf->config_cb.private = cb->private;
  }
    /*
@@ -292,6 +305,13 @@ static int ifcvf_request_irq(struct 
ifcvf_adapter *adapter)

  struct ifcvf_hw *vf = >vf;
  int vector, i, ret, irq;
  +    snprintf(vf->config_msix_name, 256, "ifcvf[%s]-config\n",
+    pci_name(pdev));
+    vector = 0;
+    irq = pci_irq_vector(pdev, vector);
+    ret = devm_request_irq(>dev, irq,
+   ifcvf_config_changed, 0,
+   vf->config_msix_name, vf);
    for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
  snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/2] vdpa: Support config interrupt in vhost_vdpa

2020-04-25 Thread Zhu Lingshan



On 4/26/2020 11:07 AM, Jason Wang wrote:


On 2020/4/24 下午6:04, Zhu Lingshan wrote:

This commit implements config interrupt support in
vhost_vdpa layer.

Signed-off-by: Zhu Lingshan 

Signed-off-by: Zhu Lingshan 



One should be sufficient.



---
  drivers/vhost/vdpa.c | 53 


  include/uapi/linux/vhost.h   |  2 ++
  include/uapi/linux/vhost_types.h |  2 ++
  3 files changed, 57 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 421f02a..f1f69bf 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -21,6 +21,7 @@
  #include 
  #include 
  #include 
+#include 
    #include "vhost.h"
  @@ -70,6 +71,7 @@ struct vhost_vdpa {
  int nvqs;
  int virtio_id;
  int minor;
+    struct eventfd_ctx *config_ctx;
  };
    static DEFINE_IDA(vhost_vdpa_ida);
@@ -101,6 +103,17 @@ static irqreturn_t vhost_vdpa_virtqueue_cb(void 
*private)

  return IRQ_HANDLED;
  }
  +static irqreturn_t vhost_vdpa_config_cb(void *private)
+{
+    struct vhost_vdpa *v = private;
+    struct eventfd_ctx *config_ctx = v->config_ctx;
+
+    if (config_ctx)
+    eventfd_signal(config_ctx, 1);
+
+    return IRQ_HANDLED;
+}
+
  static void vhost_vdpa_reset(struct vhost_vdpa *v)
  {
  struct vdpa_device *vdpa = v->vdpa;
@@ -288,6 +301,42 @@ static long vhost_vdpa_get_vring_num(struct 
vhost_vdpa *v, u16 __user *argp)

  return 0;
  }
  +static void vhost_vdpa_config_put(struct vhost_vdpa *v)
+{
+    if (v->config_ctx)
+    eventfd_ctx_put(v->config_ctx);
+}
+
+static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 
__user *argp)

+{
+    struct vdpa_callback cb;
+    vhost_config_file file;
+    struct eventfd_ctx *ctx;
+
+    cb.callback = vhost_vdpa_config_cb;
+    cb.private = v->vdpa;
+    if (copy_from_user(, argp, sizeof(file)))
+    return  -EFAULT;
+
+    if (file.fd == -1) {
+    vhost_vdpa_config_put(v);
+    v->config_ctx = NULL;
+    return PTR_ERR(v->config_ctx);
+    }
+
+    ctx = eventfd_ctx_fdget(file.fd);



You may simply did ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd);

Then there's no need for the specialized action for -1 above.


OK





+    swap(ctx, v->config_ctx);
+
+    if (!IS_ERR_OR_NULL(ctx))
+    eventfd_ctx_put(ctx);
+
+    if (IS_ERR(v->config_ctx))
+    return PTR_ERR(v->config_ctx);
+
+ v->vdpa->config->set_config_cb(v->vdpa, );
+
+    return 0;
+}
  static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned 
int cmd,

 void __user *argp)
  {
@@ -398,6 +447,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file 
*filep,

  case VHOST_SET_LOG_FD:
  r = -ENOIOCTLCMD;
  break;
+    case VHOST_VDPA_SET_CONFIG_CALL:
+    r = vhost_vdpa_set_config_call(v, argp);
+    break;
  default:
  r = vhost_dev_ioctl(>vdev, cmd, argp);
  if (r == -ENOIOCTLCMD)
@@ -734,6 +786,7 @@ static int vhost_vdpa_release(struct inode 
*inode, struct file *filep)

  vhost_dev_stop(>vdev);
  vhost_vdpa_iotlb_free(v);
  vhost_vdpa_free_domain(v);
+    vhost_vdpa_config_put(v);
  vhost_dev_cleanup(>vdev);
  kfree(v->vdev.vqs);
  mutex_unlock(>mutex);
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 9fe72e4..c474a35 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -140,4 +140,6 @@
  /* Get the max ring size. */
  #define VHOST_VDPA_GET_VRING_NUM _IOR(VHOST_VIRTIO, 0x76, __u16)
  +/* Set event fd for config interrupt*/
+#define VHOST_VDPA_SET_CONFIG_CALL _IOW(VHOST_VIRTIO, 0x77, 
vhost_config_file)

  #endif
diff --git a/include/uapi/linux/vhost_types.h 
b/include/uapi/linux/vhost_types.h

index 669457c..6759aefb 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -27,6 +27,8 @@ struct vhost_vring_file {
    };
  +typedef struct vhost_vring_file vhost_config_file;
+



I wonder maybe this is the best approach. Maybe it's better to use 
vhost_vring_file or just use a int (but need document the -1 action).


Thanks



OK, I used a typedef to avoid confusion of placing a vhost_vring_file in the 
device struct than in a vring. I will use an u32 in next version and
add a macro VHOST_FILE_UNBIND to document itself.

Thanks,
Zhu Lingshan


  struct vhost_vring_addr {
  unsigned int index;
  /* Option flags. */


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/2] vdpa: implement config interrupt in IFCVF

2020-04-25 Thread Jason Wang



On 2020/4/24 下午6:04, Zhu Lingshan wrote:

This commit implements config interrupt support
in IFC VF

Signed-off-by: Zhu Lingshan 
---
  drivers/vdpa/ifcvf/ifcvf_base.c |  3 +++
  drivers/vdpa/ifcvf/ifcvf_base.h |  2 ++
  drivers/vdpa/ifcvf/ifcvf_main.c | 22 +-
  3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
index b61b06e..c825d99 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.c
+++ b/drivers/vdpa/ifcvf/ifcvf_base.c
@@ -185,6 +185,9 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
  
  void ifcvf_reset(struct ifcvf_hw *hw)

  {
+   hw->config_cb.callback = NULL;
+   hw->config_cb.private = NULL;
+
ifcvf_set_status(hw, 0);
/* flush set_status, make sure VF is stopped, reset */
ifcvf_get_status(hw);
diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index e803070..76928b0 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -81,6 +81,8 @@ struct ifcvf_hw {
void __iomem *net_cfg;
struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
void __iomem * const *base;
+   char config_msix_name[256];
+   struct vdpa_callback config_cb;
  };
  
  struct ifcvf_adapter {

diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index 8d54dc5..f7baeca 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -18,6 +18,16 @@
  #define DRIVER_AUTHOR   "Intel Corporation"
  #define IFCVF_DRIVER_NAME   "ifcvf"
  
+static irqreturn_t ifcvf_config_changed(int irq, void *arg)

+{
+   struct ifcvf_hw *vf = arg;
+
+   if (vf->config_cb.callback)
+   return vf->config_cb.callback(vf->config_cb.private);
+
+   return IRQ_HANDLED;



So it looks to me the current support of VIRTIO_NET_F_STATUS is broken 
without this patch.


We probably need to patch to disable it.

Thanks



+}
+
  static irqreturn_t ifcvf_intr_handler(int irq, void *arg)
  {
struct vring_info *vring = arg;
@@ -256,7 +266,10 @@ static void ifcvf_vdpa_set_config(struct vdpa_device 
*vdpa_dev,
  static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
 struct vdpa_callback *cb)
  {
-   /* We don't support config interrupt */
+   struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
+
+   vf->config_cb.callback = cb->callback;
+   vf->config_cb.private = cb->private;
  }
  
  /*

@@ -292,6 +305,13 @@ static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
struct ifcvf_hw *vf = >vf;
int vector, i, ret, irq;
  
+	snprintf(vf->config_msix_name, 256, "ifcvf[%s]-config\n",

+   pci_name(pdev));
+   vector = 0;
+   irq = pci_irq_vector(pdev, vector);
+   ret = devm_request_irq(>dev, irq,
+  ifcvf_config_changed, 0,
+  vf->config_msix_name, vf);
  
  	for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {

snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/2] vdpa: Support config interrupt in vhost_vdpa

2020-04-25 Thread Jason Wang



On 2020/4/24 下午6:04, Zhu Lingshan wrote:

This commit implements config interrupt support in
vhost_vdpa layer.

Signed-off-by: Zhu Lingshan 

Signed-off-by: Zhu Lingshan 



One should be sufficient.



---
  drivers/vhost/vdpa.c | 53 
  include/uapi/linux/vhost.h   |  2 ++
  include/uapi/linux/vhost_types.h |  2 ++
  3 files changed, 57 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 421f02a..f1f69bf 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -21,6 +21,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include "vhost.h"
  
@@ -70,6 +71,7 @@ struct vhost_vdpa {

int nvqs;
int virtio_id;
int minor;
+   struct eventfd_ctx *config_ctx;
  };
  
  static DEFINE_IDA(vhost_vdpa_ida);

@@ -101,6 +103,17 @@ static irqreturn_t vhost_vdpa_virtqueue_cb(void *private)
return IRQ_HANDLED;
  }
  
+static irqreturn_t vhost_vdpa_config_cb(void *private)

+{
+   struct vhost_vdpa *v = private;
+   struct eventfd_ctx *config_ctx = v->config_ctx;
+
+   if (config_ctx)
+   eventfd_signal(config_ctx, 1);
+
+   return IRQ_HANDLED;
+}
+
  static void vhost_vdpa_reset(struct vhost_vdpa *v)
  {
struct vdpa_device *vdpa = v->vdpa;
@@ -288,6 +301,42 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, 
u16 __user *argp)
return 0;
  }
  
+static void vhost_vdpa_config_put(struct vhost_vdpa *v)

+{
+   if (v->config_ctx)
+   eventfd_ctx_put(v->config_ctx);
+}
+
+static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp)
+{
+   struct vdpa_callback cb;
+   vhost_config_file file;
+   struct eventfd_ctx *ctx;
+
+   cb.callback = vhost_vdpa_config_cb;
+   cb.private = v->vdpa;
+   if (copy_from_user(, argp, sizeof(file)))
+   return  -EFAULT;
+
+   if (file.fd == -1) {
+   vhost_vdpa_config_put(v);
+   v->config_ctx = NULL;
+   return PTR_ERR(v->config_ctx);
+   }
+
+   ctx = eventfd_ctx_fdget(file.fd);



You may simply did ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd);

Then there's no need for the specialized action for -1 above.



+   swap(ctx, v->config_ctx);
+
+   if (!IS_ERR_OR_NULL(ctx))
+   eventfd_ctx_put(ctx);
+
+   if (IS_ERR(v->config_ctx))
+   return PTR_ERR(v->config_ctx);
+
+   v->vdpa->config->set_config_cb(v->vdpa, );
+
+   return 0;
+}
  static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
   void __user *argp)
  {
@@ -398,6 +447,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
case VHOST_SET_LOG_FD:
r = -ENOIOCTLCMD;
break;
+   case VHOST_VDPA_SET_CONFIG_CALL:
+   r = vhost_vdpa_set_config_call(v, argp);
+   break;
default:
r = vhost_dev_ioctl(>vdev, cmd, argp);
if (r == -ENOIOCTLCMD)
@@ -734,6 +786,7 @@ static int vhost_vdpa_release(struct inode *inode, struct 
file *filep)
vhost_dev_stop(>vdev);
vhost_vdpa_iotlb_free(v);
vhost_vdpa_free_domain(v);
+   vhost_vdpa_config_put(v);
vhost_dev_cleanup(>vdev);
kfree(v->vdev.vqs);
mutex_unlock(>mutex);
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 9fe72e4..c474a35 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -140,4 +140,6 @@
  /* Get the max ring size. */
  #define VHOST_VDPA_GET_VRING_NUM  _IOR(VHOST_VIRTIO, 0x76, __u16)
  
+/* Set event fd for config interrupt*/

+#define VHOST_VDPA_SET_CONFIG_CALL _IOW(VHOST_VIRTIO, 0x77, 
vhost_config_file)
  #endif
diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
index 669457c..6759aefb 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -27,6 +27,8 @@ struct vhost_vring_file {
  
  };
  
+typedef struct vhost_vring_file vhost_config_file;

+



I wonder maybe this is the best approach. Maybe it's better to use 
vhost_vring_file or just use a int (but need document the -1 action).


Thanks



  struct vhost_vring_addr {
unsigned int index;
/* Option flags. */


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

2020-04-25 Thread Andy Lutomirski

On Sat, Apr 25, 2020 at 1:23 PM Joerg Roedel  wrote:
>
> On Sat, Apr 25, 2020 at 12:47:31PM -0700, Andy Lutomirski wrote:
> > I assume the race you mean is:
> >
> > #VC
> > Immediate NMI before IST gets shifted
> > #VC
> >
> > Kaboom.
> >
> > How are you dealing with this?  Ultimately, I think that NMI will need
> > to turn off IST before engaging in any funny business. Let me ponder
> > this a bit.
>
> Right, I dealt with that by unconditionally shifting/unshifting the #VC IST 
> entry
> in do_nmi() (thanks to Davin Kaplan for the idea). It might cause
> one of the IST stacks to be unused during nesting, but that is fine. The
> stack memory for #VC is only allocated when SEV-ES is active (in an
> SEV-ES VM).

Blech.  It probably works, but still, yuck.  It's a bit sad that we
seem to be growing more and more poorly designed happens-anywhere
exception types at an alarming rate.  We seem to have #NM, #MC, #VC,
#HV, and #DB.  This doesn't really scale.

--Andy
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

2020-04-25 Thread Joerg Roedel

On Sat, Apr 25, 2020 at 12:47:31PM -0700, Andy Lutomirski wrote:
> I assume the race you mean is:
> 
> #VC
> Immediate NMI before IST gets shifted
> #VC
> 
> Kaboom.
> 
> How are you dealing with this?  Ultimately, I think that NMI will need
> to turn off IST before engaging in any funny business. Let me ponder
> this a bit.

Right, I dealt with that by unconditionally shifting/unshifting the #VC IST 
entry
in do_nmi() (thanks to Davin Kaplan for the idea). It might cause
one of the IST stacks to be unused during nesting, but that is fine. The
stack memory for #VC is only allocated when SEV-ES is active (in an
SEV-ES VM).

Regards,

Joerg
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

2020-04-25 Thread Andy Lutomirski



> On Apr 25, 2020, at 12:10 PM, Joerg Roedel  wrote:
> 
> On Sat, Apr 25, 2020 at 11:15:35AM -0700, Andy Lutomirski wrote:
>> shift_ist is gross.  What's it for?  If it's not needed, I'd rather
>> not use it, and I eventually want to get rid of it for #DB as well.
> 
> The #VC handler needs to be able to nest, there is no way around that
> for various reasons, the two most important ones are:
> 
>1. The #VC -> NMI -> #VC case. #VCs can happen in the NMI
>   handler, for example (but not exclusivly) for RDPMC.
> 
>2. In case of an error the #VC handler needs to print out error
>   information by calling one of the printk wrappers. Those will
>   end up doing IO to some console/serial port/whatever which
>   will also cause #VC exceptions to emulate the access to the
>   output devices.
> 
> Using shift_ist is perfect for that, the only problem is the race
> condition with the NMI handler, as shift_ist does not work well with
> exceptions that can also trigger within the NMI handler. But I have
> taken care of that for #VC.
> 

I assume the race you mean is:

#VC
Immediate NMI before IST gets shifted
#VC

Kaboom.

How are you dealing with this?  Ultimately, I think that NMI will need to turn 
off IST before engaging in any funny business. Let me ponder this a bit.

> 
> Regards,
> 
>Joerg
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

2020-04-25 Thread Joerg Roedel

On Sat, Apr 25, 2020 at 11:15:35AM -0700, Andy Lutomirski wrote:
> shift_ist is gross.  What's it for?  If it's not needed, I'd rather
> not use it, and I eventually want to get rid of it for #DB as well.

The #VC handler needs to be able to nest, there is no way around that
for various reasons, the two most important ones are:

1. The #VC -> NMI -> #VC case. #VCs can happen in the NMI
   handler, for example (but not exclusivly) for RDPMC.

2. In case of an error the #VC handler needs to print out error
   information by calling one of the printk wrappers. Those will
   end up doing IO to some console/serial port/whatever which
   will also cause #VC exceptions to emulate the access to the
   output devices.

Using shift_ist is perfect for that, the only problem is the race
condition with the NMI handler, as shift_ist does not work well with
exceptions that can also trigger within the NMI handler. But I have
taken care of that for #VC.

Regards,

Joerg

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

2020-04-25 Thread Andy Lutomirski

On Sat, Apr 25, 2020 at 5:49 AM Joerg Roedel  wrote:
>
> Hi Dave,
>
> On Fri, Apr 24, 2020 at 03:53:09PM -0700, Dave Hansen wrote:
> > Ahh, so any instruction that can have an instruction intercept set
> > potentially needs to be able to tolerate a #VC?  Those instruction
> > intercepts are under the control of the (untrusted relative to the
> > guest) hypervisor, right?
> >
> > >From the main sev-es series:
> >
> > +#ifdef CONFIG_AMD_MEM_ENCRYPT
> > +idtentry vmm_communication do_vmm_communicationhas_error_code=1
> > +#endif
>
> The next version of the patch-set (which I will hopefully have ready
> next week) will have this changed. The #VC exception handler uses an IST
> stack and is set to paranoid=1 and shift_ist. The IST stacks for the #VC
> handler are only allocated when SEV-ES is active.

shift_ist is gross.  What's it for?  If it's not needed, I'd rather
not use it, and I eventually want to get rid of it for #DB as well.

--Andy
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

2020-04-25 Thread Joerg Roedel

Hi Dave,

On Fri, Apr 24, 2020 at 03:53:09PM -0700, Dave Hansen wrote:
> Ahh, so any instruction that can have an instruction intercept set
> potentially needs to be able to tolerate a #VC?  Those instruction
> intercepts are under the control of the (untrusted relative to the
> guest) hypervisor, right?
> 
> >From the main sev-es series:
> 
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> +idtentry vmm_communication do_vmm_communicationhas_error_code=1
> +#endif

The next version of the patch-set (which I will hopefully have ready
next week) will have this changed. The #VC exception handler uses an IST
stack and is set to paranoid=1 and shift_ist. The IST stacks for the #VC
handler are only allocated when SEV-ES is active.

> That's a fun point because it means that the (untrusted) hypervisor can
> cause endless faults.  I *guess* we have mitigation for this with our
> stack guard pages, but it's still a bit nasty that the hypervisor can
> arbitrarily land a guest in the double-fault handler.
> 
> It just all seems a bit weak for the hypervisor to be considered
> untrusted.  But, it's _certainly_ a steep in the right direction from SEV.

Yeah, a malicious hypervisor can do bad things to an SEV-ES VM, but it
can't easily steal its secrets from memory or registers. The #VC handler
does its best to just crash the VM if unexpected hypervisor behavior is
detected.

Regards,

Joerg
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

2020-04-25 Thread Joerg Roedel

Hi Mike,

On Fri, Apr 24, 2020 at 02:03:16PM -0700, Mike Stunes wrote:
> I needed to allow RDTSC(P) from userspace and in early boot in order to
> get userspace started properly. Patch below.

Thanks, but this is not needed anymore. I removed the vc_context_filter
from the code. The emulation code is now capable of safely handling any
exception from user-space.

Regards,

Joerg

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/2] vdpa: implement config interrupt in IFCVF

Re: [PATCH 1/2] vdpa: Support config interrupt in vhost_vdpa

Re: [PATCH 2/2] vdpa: implement config interrupt in IFCVF

Re: [PATCH 1/2] vdpa: Support config interrupt in vhost_vdpa

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

Re: [PATCH] Allow RDTSC and RDTSCP from userspace

11 matches

Site Navigation

Mail list logo

Footer information