Re: [PATCH v15 4/4] vhost-vdpa: Add support for vIOMMU.

2023-03-24 Thread Jason Wang
On Thu, Mar 23, 2023 at 4:41 PM Cindy Lu  wrote:
>
> On Thu, Mar 23, 2023 at 11:47 AM Jason Wang  wrote:
> >
> > On Tue, Mar 21, 2023 at 10:24 PM Cindy Lu  wrote:
> > >
> > > 1. The vIOMMU support will make vDPA can work in IOMMU mode. This
> > > will fix security issues while using the no-IOMMU mode.
> > > To support this feature we need to add new functions for IOMMU MR adds and
> > > deletes.
> > >
> > > Also since the SVQ does not support vIOMMU yet, add the check for IOMMU
> > > in vhost_vdpa_dev_start, if the SVQ and IOMMU enable at the same time
> > > the function will return fail.
> > >
> > > 2. Skip the iova_max check vhost_vdpa_listener_skipped_section(). While
> > > MR is IOMMU, move this check to vhost_vdpa_iommu_map_notify()
> > >
> > > Verified in vp_vdpa and vdpa_sim_net driver
> > >
> > > Signed-off-by: Cindy Lu 
> > > ---
> > >  hw/virtio/trace-events |   2 +-
> > >  hw/virtio/vhost-vdpa.c | 159 ++---
> > >  include/hw/virtio/vhost-vdpa.h |  11 +++
> > >  3 files changed, 161 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > > index 8f8d05cf9b..de4da2c65c 100644
> > > --- a/hw/virtio/trace-events
> > > +++ b/hw/virtio/trace-events
> > > @@ -33,7 +33,7 @@ vhost_user_create_notifier(int idx, void *n) "idx:%d 
> > > n:%p"
> > >  vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, 
> > > uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, uint8_t type) 
> > > "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" 
> > > size: 0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 0x%"PRIx8" type: %"PRIu8
> > >  vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t 
> > > asid, uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d 
> > > msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" 
> > > type: %"PRIu8
> > >  vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, 
> > > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > > -vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t 
> > > type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > > +vhost_vdpa_iotlb_batch_end_once(void *v, int fd, uint32_t msg_type, 
> > > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > >  vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t 
> > > llend, void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 
> > > 0x%"PRIx64" vaddr: %p read-only: %d"
> > >  vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t 
> > > llend) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64
> > >  vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 
> > > 0x%"PRIx8
> > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > > index 0c8c37e786..39720d12a6 100644
> > > --- a/hw/virtio/vhost-vdpa.c
> > > +++ b/hw/virtio/vhost-vdpa.c
> > > @@ -26,6 +26,7 @@
> > >  #include "cpu.h"
> > >  #include "trace.h"
> > >  #include "qapi/error.h"
> > > +#include "hw/virtio/virtio-access.h"
> > >
> > >  /*
> > >   * Return one past the end of the end of section. Be careful with 
> > > uint64_t
> > > @@ -60,13 +61,21 @@ static bool 
> > > vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
> > >   iova_min, section->offset_within_address_space);
> > >  return true;
> > >  }
> > > +/*
> > > + * While using vIOMMU, sometimes the section will be larger than 
> > > iova_max,
> > > + * but the memory that actually maps is smaller, so move the check to
> > > + * function vhost_vdpa_iommu_map_notify(). That function will use 
> > > the actual
> > > + * size that maps to the kernel
> > > + */
> > >
> > > -llend = vhost_vdpa_section_end(section);
> > > -if (int128_gt(llend, int128_make64(iova_max))) {
> > > -error_report("RAM section out of device range (max=0x%" PRIx64
> > > - ", end addr=0x%" PRIx64 ")",
> > > - iova_max, int128_get64(llend));
> > > -return true;
> > > +if (!memory_region_is_iommu(section->mr)) {
> > > +llend = vhost_vdpa_section_end(section);
> > > +if (int128_gt(llend, int128_make64(iova_max))) {
> > > +error_report("RAM section out of device range (max=0x%" 
> > > PRIx64
> > > + ", end addr=0x%" PRIx64 ")",
> > > + iova_max, int128_get64(llend));
> > > +return true;
> > > +}
> > >  }
> > >
> > >  return false;
> > > @@ -158,9 +167,8 @@ static void vhost_vdpa_iotlb_batch_begin_once(struct 
> > > vhost_vdpa *v)
> > >  v->iotlb_batch_begin_sent = true;
> > >  }
> > >
> > > -static void vhost_vdpa_listener_commit(MemoryListener *listener)
> > > +static void vhost_vdpa_iotlb_batch_end_once(struct vhost_vdpa *v)
> > >  {
> > > -struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, 
> > > list

Re: [PATCH v15 4/4] vhost-vdpa: Add support for vIOMMU.

2023-03-24 Thread Jason Wang
On Fri, Mar 24, 2023 at 11:00 AM Cindy Lu  wrote:
>
> On Fri, Mar 24, 2023 at 10:49 AM Jason Wang  wrote:
> >
> > On Thu, Mar 23, 2023 at 4:41 PM Cindy Lu  wrote:
> > >
> > > On Thu, Mar 23, 2023 at 11:47 AM Jason Wang  wrote:
> > > >
> > > > On Tue, Mar 21, 2023 at 10:24 PM Cindy Lu  wrote:
> > > > >
> > > > > 1. The vIOMMU support will make vDPA can work in IOMMU mode. This
> > > > > will fix security issues while using the no-IOMMU mode.
> > > > > To support this feature we need to add new functions for IOMMU MR 
> > > > > adds and
> > > > > deletes.
> > > > >
> > > > > Also since the SVQ does not support vIOMMU yet, add the check for 
> > > > > IOMMU
> > > > > in vhost_vdpa_dev_start, if the SVQ and IOMMU enable at the same time
> > > > > the function will return fail.
> > > > >
> > > > > 2. Skip the iova_max check vhost_vdpa_listener_skipped_section(). 
> > > > > While
> > > > > MR is IOMMU, move this check to vhost_vdpa_iommu_map_notify()
> > > > >
> > > > > Verified in vp_vdpa and vdpa_sim_net driver
> > > > >
> > > > > Signed-off-by: Cindy Lu 
> > > > > ---
> > > > >  hw/virtio/trace-events |   2 +-
> > > > >  hw/virtio/vhost-vdpa.c | 159 
> > > > > ++---
> > > > >  include/hw/virtio/vhost-vdpa.h |  11 +++
> > > > >  3 files changed, 161 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > > > > index 8f8d05cf9b..de4da2c65c 100644
> > > > > --- a/hw/virtio/trace-events
> > > > > +++ b/hw/virtio/trace-events
> > > > > @@ -33,7 +33,7 @@ vhost_user_create_notifier(int idx, void *n) 
> > > > > "idx:%d n:%p"
> > > > >  vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t 
> > > > > asid, uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, 
> > > > > uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" 
> > > > > iova: 0x%"PRIx64" size: 0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 
> > > > > 0x%"PRIx8" type: %"PRIu8
> > > > >  vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t 
> > > > > asid, uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d 
> > > > > msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 
> > > > > 0x%"PRIx64" type: %"PRIu8
> > > > >  vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, 
> > > > > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > > > > -vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, 
> > > > > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > > > > +vhost_vdpa_iotlb_batch_end_once(void *v, int fd, uint32_t msg_type, 
> > > > > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > > > >  vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t 
> > > > > llend, void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 
> > > > > 0x%"PRIx64" vaddr: %p read-only: %d"
> > > > >  vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t 
> > > > > llend) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64
> > > > >  vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 
> > > > > 0x%"PRIx8
> > > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > > > > index 0c8c37e786..39720d12a6 100644
> > > > > --- a/hw/virtio/vhost-vdpa.c
> > > > > +++ b/hw/virtio/vhost-vdpa.c
> > > > > @@ -26,6 +26,7 @@
> > > > >  #include "cpu.h"
> > > > >  #include "trace.h"
> > > > >  #include "qapi/error.h"
> > > > > +#include "hw/virtio/virtio-access.h"
> > > > >
> > > > >  /*
> > > > >   * Return one past the end of the end of section. Be careful with 
> > > > > uint64_t
> > > > > @@ -60,13 +61,21 @@ static bool 
> > > > > vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
> > > > >   iova_min, section->offset_within_address_space);
> > > > >  return true;
> > > > >  }
> > > > > +/*
> > > > > + * While using vIOMMU, sometimes the section will be larger than 
> > > > > iova_max,
> > > > > + * but the memory that actually maps is smaller, so move the 
> > > > > check to
> > > > > + * function vhost_vdpa_iommu_map_notify(). That function will 
> > > > > use the actual
> > > > > + * size that maps to the kernel
> > > > > + */
> > > > >
> > > > > -llend = vhost_vdpa_section_end(section);
> > > > > -if (int128_gt(llend, int128_make64(iova_max))) {
> > > > > -error_report("RAM section out of device range (max=0x%" 
> > > > > PRIx64
> > > > > - ", end addr=0x%" PRIx64 ")",
> > > > > - iova_max, int128_get64(llend));
> > > > > -return true;
> > > > > +if (!memory_region_is_iommu(section->mr)) {
> > > > > +llend = vhost_vdpa_section_end(section);
> > > > > +if (int128_gt(llend, int128_make64(iova_max))) {
> > > > > +error_report("RAM section out of device range (max=0x%" 
> > > > > PRIx64
> > > > > + ", end addr=0x%" PRIx64 ")",

Re: [PATCH v15 4/4] vhost-vdpa: Add support for vIOMMU.

2023-03-23 Thread Cindy Lu
On Fri, Mar 24, 2023 at 10:49 AM Jason Wang  wrote:
>
> On Thu, Mar 23, 2023 at 4:41 PM Cindy Lu  wrote:
> >
> > On Thu, Mar 23, 2023 at 11:47 AM Jason Wang  wrote:
> > >
> > > On Tue, Mar 21, 2023 at 10:24 PM Cindy Lu  wrote:
> > > >
> > > > 1. The vIOMMU support will make vDPA can work in IOMMU mode. This
> > > > will fix security issues while using the no-IOMMU mode.
> > > > To support this feature we need to add new functions for IOMMU MR adds 
> > > > and
> > > > deletes.
> > > >
> > > > Also since the SVQ does not support vIOMMU yet, add the check for IOMMU
> > > > in vhost_vdpa_dev_start, if the SVQ and IOMMU enable at the same time
> > > > the function will return fail.
> > > >
> > > > 2. Skip the iova_max check vhost_vdpa_listener_skipped_section(). While
> > > > MR is IOMMU, move this check to vhost_vdpa_iommu_map_notify()
> > > >
> > > > Verified in vp_vdpa and vdpa_sim_net driver
> > > >
> > > > Signed-off-by: Cindy Lu 
> > > > ---
> > > >  hw/virtio/trace-events |   2 +-
> > > >  hw/virtio/vhost-vdpa.c | 159 ++---
> > > >  include/hw/virtio/vhost-vdpa.h |  11 +++
> > > >  3 files changed, 161 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > > > index 8f8d05cf9b..de4da2c65c 100644
> > > > --- a/hw/virtio/trace-events
> > > > +++ b/hw/virtio/trace-events
> > > > @@ -33,7 +33,7 @@ vhost_user_create_notifier(int idx, void *n) "idx:%d 
> > > > n:%p"
> > > >  vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t 
> > > > asid, uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, 
> > > > uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 
> > > > 0x%"PRIx64" size: 0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 0x%"PRIx8" type: 
> > > > %"PRIu8
> > > >  vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t 
> > > > asid, uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d 
> > > > msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" 
> > > > type: %"PRIu8
> > > >  vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, 
> > > > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > > > -vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t 
> > > > type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > > > +vhost_vdpa_iotlb_batch_end_once(void *v, int fd, uint32_t msg_type, 
> > > > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > > >  vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t 
> > > > llend, void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 
> > > > 0x%"PRIx64" vaddr: %p read-only: %d"
> > > >  vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t 
> > > > llend) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64
> > > >  vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 
> > > > 0x%"PRIx8
> > > > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > > > index 0c8c37e786..39720d12a6 100644
> > > > --- a/hw/virtio/vhost-vdpa.c
> > > > +++ b/hw/virtio/vhost-vdpa.c
> > > > @@ -26,6 +26,7 @@
> > > >  #include "cpu.h"
> > > >  #include "trace.h"
> > > >  #include "qapi/error.h"
> > > > +#include "hw/virtio/virtio-access.h"
> > > >
> > > >  /*
> > > >   * Return one past the end of the end of section. Be careful with 
> > > > uint64_t
> > > > @@ -60,13 +61,21 @@ static bool 
> > > > vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
> > > >   iova_min, section->offset_within_address_space);
> > > >  return true;
> > > >  }
> > > > +/*
> > > > + * While using vIOMMU, sometimes the section will be larger than 
> > > > iova_max,
> > > > + * but the memory that actually maps is smaller, so move the check 
> > > > to
> > > > + * function vhost_vdpa_iommu_map_notify(). That function will use 
> > > > the actual
> > > > + * size that maps to the kernel
> > > > + */
> > > >
> > > > -llend = vhost_vdpa_section_end(section);
> > > > -if (int128_gt(llend, int128_make64(iova_max))) {
> > > > -error_report("RAM section out of device range (max=0x%" PRIx64
> > > > - ", end addr=0x%" PRIx64 ")",
> > > > - iova_max, int128_get64(llend));
> > > > -return true;
> > > > +if (!memory_region_is_iommu(section->mr)) {
> > > > +llend = vhost_vdpa_section_end(section);
> > > > +if (int128_gt(llend, int128_make64(iova_max))) {
> > > > +error_report("RAM section out of device range (max=0x%" 
> > > > PRIx64
> > > > + ", end addr=0x%" PRIx64 ")",
> > > > + iova_max, int128_get64(llend));
> > > > +return true;
> > > > +}
> > > >  }
> > > >
> > > >  return false;
> > > > @@ -158,9 +167,8 @@ static void 
> > > > vhost_vdpa_iotlb_batch_begin_once(struct vhost_vdpa *v)
> > > >  v->io

Re: [PATCH v15 4/4] vhost-vdpa: Add support for vIOMMU.

2023-03-23 Thread Cindy Lu
On Thu, Mar 23, 2023 at 11:47 AM Jason Wang  wrote:
>
> On Tue, Mar 21, 2023 at 10:24 PM Cindy Lu  wrote:
> >
> > 1. The vIOMMU support will make vDPA can work in IOMMU mode. This
> > will fix security issues while using the no-IOMMU mode.
> > To support this feature we need to add new functions for IOMMU MR adds and
> > deletes.
> >
> > Also since the SVQ does not support vIOMMU yet, add the check for IOMMU
> > in vhost_vdpa_dev_start, if the SVQ and IOMMU enable at the same time
> > the function will return fail.
> >
> > 2. Skip the iova_max check vhost_vdpa_listener_skipped_section(). While
> > MR is IOMMU, move this check to vhost_vdpa_iommu_map_notify()
> >
> > Verified in vp_vdpa and vdpa_sim_net driver
> >
> > Signed-off-by: Cindy Lu 
> > ---
> >  hw/virtio/trace-events |   2 +-
> >  hw/virtio/vhost-vdpa.c | 159 ++---
> >  include/hw/virtio/vhost-vdpa.h |  11 +++
> >  3 files changed, 161 insertions(+), 11 deletions(-)
> >
> > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > index 8f8d05cf9b..de4da2c65c 100644
> > --- a/hw/virtio/trace-events
> > +++ b/hw/virtio/trace-events
> > @@ -33,7 +33,7 @@ vhost_user_create_notifier(int idx, void *n) "idx:%d n:%p"
> >  vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, 
> > uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, uint8_t type) 
> > "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 
> > 0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 0x%"PRIx8" type: %"PRIu8
> >  vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, 
> > uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: 
> > %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8
> >  vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, 
> > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > -vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t 
> > type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> > +vhost_vdpa_iotlb_batch_end_once(void *v, int fd, uint32_t msg_type, 
> > uint8_t type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> >  vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, 
> > void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" 
> > vaddr: %p read-only: %d"
> >  vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) 
> > "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64
> >  vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 0c8c37e786..39720d12a6 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -26,6 +26,7 @@
> >  #include "cpu.h"
> >  #include "trace.h"
> >  #include "qapi/error.h"
> > +#include "hw/virtio/virtio-access.h"
> >
> >  /*
> >   * Return one past the end of the end of section. Be careful with uint64_t
> > @@ -60,13 +61,21 @@ static bool 
> > vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
> >   iova_min, section->offset_within_address_space);
> >  return true;
> >  }
> > +/*
> > + * While using vIOMMU, sometimes the section will be larger than 
> > iova_max,
> > + * but the memory that actually maps is smaller, so move the check to
> > + * function vhost_vdpa_iommu_map_notify(). That function will use the 
> > actual
> > + * size that maps to the kernel
> > + */
> >
> > -llend = vhost_vdpa_section_end(section);
> > -if (int128_gt(llend, int128_make64(iova_max))) {
> > -error_report("RAM section out of device range (max=0x%" PRIx64
> > - ", end addr=0x%" PRIx64 ")",
> > - iova_max, int128_get64(llend));
> > -return true;
> > +if (!memory_region_is_iommu(section->mr)) {
> > +llend = vhost_vdpa_section_end(section);
> > +if (int128_gt(llend, int128_make64(iova_max))) {
> > +error_report("RAM section out of device range (max=0x%" PRIx64
> > + ", end addr=0x%" PRIx64 ")",
> > + iova_max, int128_get64(llend));
> > +return true;
> > +}
> >  }
> >
> >  return false;
> > @@ -158,9 +167,8 @@ static void vhost_vdpa_iotlb_batch_begin_once(struct 
> > vhost_vdpa *v)
> >  v->iotlb_batch_begin_sent = true;
> >  }
> >
> > -static void vhost_vdpa_listener_commit(MemoryListener *listener)
> > +static void vhost_vdpa_iotlb_batch_end_once(struct vhost_vdpa *v)
> >  {
> > -struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, 
> > listener);
> >  struct vhost_dev *dev = v->dev;
> >  struct vhost_msg_v2 msg = {};
> >  int fd = v->device_fd;
> > @@ -176,7 +184,7 @@ static void vhost_vdpa_listener_commit(MemoryListener 
> > *listener)
> >  msg.type = v->msg_type;
> >  msg.iotlb.type = VHOST_IOTLB_BATCH_

Re: [PATCH v15 4/4] vhost-vdpa: Add support for vIOMMU.

2023-03-22 Thread Jason Wang
On Tue, Mar 21, 2023 at 10:24 PM Cindy Lu  wrote:
>
> 1. The vIOMMU support will make vDPA can work in IOMMU mode. This
> will fix security issues while using the no-IOMMU mode.
> To support this feature we need to add new functions for IOMMU MR adds and
> deletes.
>
> Also since the SVQ does not support vIOMMU yet, add the check for IOMMU
> in vhost_vdpa_dev_start, if the SVQ and IOMMU enable at the same time
> the function will return fail.
>
> 2. Skip the iova_max check vhost_vdpa_listener_skipped_section(). While
> MR is IOMMU, move this check to vhost_vdpa_iommu_map_notify()
>
> Verified in vp_vdpa and vdpa_sim_net driver
>
> Signed-off-by: Cindy Lu 
> ---
>  hw/virtio/trace-events |   2 +-
>  hw/virtio/vhost-vdpa.c | 159 ++---
>  include/hw/virtio/vhost-vdpa.h |  11 +++
>  3 files changed, 161 insertions(+), 11 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 8f8d05cf9b..de4da2c65c 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -33,7 +33,7 @@ vhost_user_create_notifier(int idx, void *n) "idx:%d n:%p"
>  vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, 
> uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, uint8_t type) 
> "vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 
> 0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 0x%"PRIx8" type: %"PRIu8
>  vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, 
> uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: 
> %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8
>  vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t 
> type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> -vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type) 
>  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
> +vhost_vdpa_iotlb_batch_end_once(void *v, int fd, uint32_t msg_type, uint8_t 
> type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
>  vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, 
> void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" 
> vaddr: %p read-only: %d"
>  vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) 
> "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64
>  vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 0c8c37e786..39720d12a6 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -26,6 +26,7 @@
>  #include "cpu.h"
>  #include "trace.h"
>  #include "qapi/error.h"
> +#include "hw/virtio/virtio-access.h"
>
>  /*
>   * Return one past the end of the end of section. Be careful with uint64_t
> @@ -60,13 +61,21 @@ static bool 
> vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
>   iova_min, section->offset_within_address_space);
>  return true;
>  }
> +/*
> + * While using vIOMMU, sometimes the section will be larger than 
> iova_max,
> + * but the memory that actually maps is smaller, so move the check to
> + * function vhost_vdpa_iommu_map_notify(). That function will use the 
> actual
> + * size that maps to the kernel
> + */
>
> -llend = vhost_vdpa_section_end(section);
> -if (int128_gt(llend, int128_make64(iova_max))) {
> -error_report("RAM section out of device range (max=0x%" PRIx64
> - ", end addr=0x%" PRIx64 ")",
> - iova_max, int128_get64(llend));
> -return true;
> +if (!memory_region_is_iommu(section->mr)) {
> +llend = vhost_vdpa_section_end(section);
> +if (int128_gt(llend, int128_make64(iova_max))) {
> +error_report("RAM section out of device range (max=0x%" PRIx64
> + ", end addr=0x%" PRIx64 ")",
> + iova_max, int128_get64(llend));
> +return true;
> +}
>  }
>
>  return false;
> @@ -158,9 +167,8 @@ static void vhost_vdpa_iotlb_batch_begin_once(struct 
> vhost_vdpa *v)
>  v->iotlb_batch_begin_sent = true;
>  }
>
> -static void vhost_vdpa_listener_commit(MemoryListener *listener)
> +static void vhost_vdpa_iotlb_batch_end_once(struct vhost_vdpa *v)
>  {
> -struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, 
> listener);
>  struct vhost_dev *dev = v->dev;
>  struct vhost_msg_v2 msg = {};
>  int fd = v->device_fd;
> @@ -176,7 +184,7 @@ static void vhost_vdpa_listener_commit(MemoryListener 
> *listener)
>  msg.type = v->msg_type;
>  msg.iotlb.type = VHOST_IOTLB_BATCH_END;
>
> -trace_vhost_vdpa_listener_commit(v, fd, msg.type, msg.iotlb.type);
> +trace_vhost_vdpa_iotlb_batch_end_once(v, fd, msg.type, msg.iotlb.type);

I suggest to keep the commit trace. The commit and batch are different
things. If you want to trace the bat

[PATCH v15 4/4] vhost-vdpa: Add support for vIOMMU.

2023-03-21 Thread Cindy Lu
1. The vIOMMU support will make vDPA can work in IOMMU mode. This
will fix security issues while using the no-IOMMU mode.
To support this feature we need to add new functions for IOMMU MR adds and
deletes.

Also since the SVQ does not support vIOMMU yet, add the check for IOMMU
in vhost_vdpa_dev_start, if the SVQ and IOMMU enable at the same time
the function will return fail.

2. Skip the iova_max check vhost_vdpa_listener_skipped_section(). While
MR is IOMMU, move this check to vhost_vdpa_iommu_map_notify()

Verified in vp_vdpa and vdpa_sim_net driver

Signed-off-by: Cindy Lu 
---
 hw/virtio/trace-events |   2 +-
 hw/virtio/vhost-vdpa.c | 159 ++---
 include/hw/virtio/vhost-vdpa.h |  11 +++
 3 files changed, 161 insertions(+), 11 deletions(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 8f8d05cf9b..de4da2c65c 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -33,7 +33,7 @@ vhost_user_create_notifier(int idx, void *n) "idx:%d n:%p"
 vhost_vdpa_dma_map(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, 
uint64_t iova, uint64_t size, uint64_t uaddr, uint8_t perm, uint8_t type) 
"vdpa:%p fd: %d msg_type: %"PRIu32" asid: %"PRIu32" iova: 0x%"PRIx64" size: 
0x%"PRIx64" uaddr: 0x%"PRIx64" perm: 0x%"PRIx8" type: %"PRIu8
 vhost_vdpa_dma_unmap(void *vdpa, int fd, uint32_t msg_type, uint32_t asid, 
uint64_t iova, uint64_t size, uint8_t type) "vdpa:%p fd: %d msg_type: %"PRIu32" 
asid: %"PRIu32" iova: 0x%"PRIx64" size: 0x%"PRIx64" type: %"PRIu8
 vhost_vdpa_listener_begin_batch(void *v, int fd, uint32_t msg_type, uint8_t 
type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
-vhost_vdpa_listener_commit(void *v, int fd, uint32_t msg_type, uint8_t type)  
"vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
+vhost_vdpa_iotlb_batch_end_once(void *v, int fd, uint32_t msg_type, uint8_t 
type)  "vdpa:%p fd: %d msg_type: %"PRIu32" type: %"PRIu8
 vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t llend, void 
*vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64" vaddr: %p 
read-only: %d"
 vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t llend) 
"vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64
 vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status: 0x%"PRIx8
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 0c8c37e786..39720d12a6 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -26,6 +26,7 @@
 #include "cpu.h"
 #include "trace.h"
 #include "qapi/error.h"
+#include "hw/virtio/virtio-access.h"
 
 /*
  * Return one past the end of the end of section. Be careful with uint64_t
@@ -60,13 +61,21 @@ static bool 
vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
  iova_min, section->offset_within_address_space);
 return true;
 }
+/*
+ * While using vIOMMU, sometimes the section will be larger than iova_max,
+ * but the memory that actually maps is smaller, so move the check to
+ * function vhost_vdpa_iommu_map_notify(). That function will use the 
actual
+ * size that maps to the kernel
+ */
 
-llend = vhost_vdpa_section_end(section);
-if (int128_gt(llend, int128_make64(iova_max))) {
-error_report("RAM section out of device range (max=0x%" PRIx64
- ", end addr=0x%" PRIx64 ")",
- iova_max, int128_get64(llend));
-return true;
+if (!memory_region_is_iommu(section->mr)) {
+llend = vhost_vdpa_section_end(section);
+if (int128_gt(llend, int128_make64(iova_max))) {
+error_report("RAM section out of device range (max=0x%" PRIx64
+ ", end addr=0x%" PRIx64 ")",
+ iova_max, int128_get64(llend));
+return true;
+}
 }
 
 return false;
@@ -158,9 +167,8 @@ static void vhost_vdpa_iotlb_batch_begin_once(struct 
vhost_vdpa *v)
 v->iotlb_batch_begin_sent = true;
 }
 
-static void vhost_vdpa_listener_commit(MemoryListener *listener)
+static void vhost_vdpa_iotlb_batch_end_once(struct vhost_vdpa *v)
 {
-struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
 struct vhost_dev *dev = v->dev;
 struct vhost_msg_v2 msg = {};
 int fd = v->device_fd;
@@ -176,7 +184,7 @@ static void vhost_vdpa_listener_commit(MemoryListener 
*listener)
 msg.type = v->msg_type;
 msg.iotlb.type = VHOST_IOTLB_BATCH_END;
 
-trace_vhost_vdpa_listener_commit(v, fd, msg.type, msg.iotlb.type);
+trace_vhost_vdpa_iotlb_batch_end_once(v, fd, msg.type, msg.iotlb.type);
 if (write(fd, &msg, sizeof(msg)) != sizeof(msg)) {
 error_report("failed to write, fd=%d, errno=%d (%s)",
  fd, errno, strerror(errno));
@@ -185,6 +193,124 @@ static void vhost_vdpa_listener_commit(MemoryListener 
*listener)
 v->iotlb_batch_begin_sent = false;
 }
 
+static void vhost_vdpa_listener_commit(MemoryListener *list