Re: [PATCH 3/4] virtio_ring: introduce a per virtqueue waitqueue

2023-01-28 Thread Michael S. Tsirkin 
On Sun, Jan 29, 2023 at 01:48:49PM +0800, Jason Wang wrote:
> On Fri, Jan 27, 2023 at 6:35 PM Michael S. Tsirkin  wrote:
> >
> > On Fri, Dec 30, 2022 at 11:43:08AM +0800, Jason Wang wrote:
> > > On Thu, Dec 29, 2022 at 4:10 PM Michael S. Tsirkin  
> > > wrote:
> > > >
> > > > On Thu, Dec 29, 2022 at 04:04:13PM +0800, Jason Wang wrote:
> > > > > On Thu, Dec 29, 2022 at 3:07 PM Michael S. Tsirkin  
> > > > > wrote:
> > > > > >
> > > > > > On Wed, Dec 28, 2022 at 07:53:08PM +0800, Jason Wang wrote:
> > > > > > > On Wed, Dec 28, 2022 at 2:34 PM Jason Wang  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > 在 2022/12/27 17:38, Michael S. Tsirkin 写道:
> > > > > > > > > On Tue, Dec 27, 2022 at 05:12:58PM +0800, Jason Wang wrote:
> > > > > > > > >> 在 2022/12/27 15:33, Michael S. Tsirkin 写道:
> > > > > > > > >>> On Tue, Dec 27, 2022 at 12:30:35PM +0800, Jason Wang wrote:
> > > > > > > > > But device is still going and will later use the buffers.
> > > > > > > > >
> > > > > > > > > Same for timeout really.
> > > > > > > >  Avoiding infinite wait/poll is one of the goals, another 
> > > > > > > >  is to sleep.
> > > > > > > >  If we think the timeout is hard, we can start from the 
> > > > > > > >  wait.
> > > > > > > > 
> > > > > > > >  Thanks
> > > > > > > > >>> If the goal is to avoid disrupting traffic while CVQ is in 
> > > > > > > > >>> use,
> > > > > > > > >>> that sounds more reasonable. E.g. someone is turning on 
> > > > > > > > >>> promisc,
> > > > > > > > >>> a spike in CPU usage might be unwelcome.
> > > > > > > > >>
> > > > > > > > >> Yes, this would be more obvious is UP is used.
> > > > > > > > >>
> > > > > > > > >>
> > > > > > > > >>> things we should be careful to address then:
> > > > > > > > >>> 1- debugging. Currently it's easy to see a warning if CPU 
> > > > > > > > >>> is stuck
> > > > > > > > >>>  in a loop for a while, and we also get a backtrace.
> > > > > > > > >>>  E.g. with this - how do we know who has the RTNL?
> > > > > > > > >>>  We need to integrate with kernel/watchdog.c for good 
> > > > > > > > >>> results
> > > > > > > > >>>  and to make sure policy is consistent.
> > > > > > > > >>
> > > > > > > > >> That's fine, will consider this.
> > > > > > >
> > > > > > > So after some investigation, it seems the watchdog.c doesn't 
> > > > > > > help. The
> > > > > > > only export helper is touch_softlockup_watchdog() which tries to 
> > > > > > > avoid
> > > > > > > triggering the lockups warning for the known slow path.
> > > > > >
> > > > > > I never said you can just use existing exporting APIs. You'll have 
> > > > > > to
> > > > > > write new ones :)
> > > > >
> > > > > Ok, I thought you wanted to trigger similar warnings as a watchdog.
> > > > >
> > > > > Btw, I wonder what kind of logic you want here. If we switch to using
> > > > > sleep, there won't be soft lockup anymore. A simple wait + timeout +
> > > > > warning seems sufficient?
> > > > >
> > > > > Thanks
> > > >
> > > > I'd like to avoid need to teach users new APIs. So watchdog setup to 
> > > > apply
> > > > to this driver. The warning can be different.
> > >
> > > Right, so it looks to me the only possible setup is the
> > > watchdog_thres. I plan to trigger the warning every watchdog_thres * 2
> > > second (as softlockup did).
> > >
> > > And I think it would still make sense to fail, we can start with a
> > > very long timeout like 1 minutes and break the device. Does this make
> > > sense?
> > >
> > > Thanks
> >
> > I'd say we need to make this manageable then.
> 
> Did you mean something like sysfs or module parameters?

No I'd say pass it with an ioctl.

> > Can't we do it normally
> > e.g. react to an interrupt to return to userspace?
> 
> I didn't get the meaning of this. Sorry.
> 
> Thanks

Standard way to handle things that can timeout and where userspace
did not supply the time is to block until an interrupt
then return EINTR. Userspace controls the timeout by
using e.g. alarm(2).


> >
> >
> >
> > > >
> > > >
> > > > > >
> > > > > > > And before the patch, we end up with a real infinite loop which 
> > > > > > > could
> > > > > > > be caught by RCU stall detector which is not the case of the 
> > > > > > > sleep.
> > > > > > > What we can do is probably do a periodic netdev_err().
> > > > > > >
> > > > > > > Thanks
> > > > > >
> > > > > > Only with a bad device.
> > > > > >
> > > > > > > > >>
> > > > > > > > >>
> > > > > > > > >>> 2- overhead. In a very common scenario when device is in 
> > > > > > > > >>> hypervisor,
> > > > > > > > >>>  programming timers etc has a very high overhead, at 
> > > > > > > > >>> bootup
> > > > > > > > >>>  lots of CVQ commands are run and slowing boot down is 
> > > > > > > > >>> not nice.
> > > > > > > > >>>  let's poll for a bit before waiting?
> > > > > > > > >>
> > > > > > > > >> Then we go back to the question of choosing a good timeout 
> > > > > > > > >> for poll. And
> > > > > > > > >> pol

Re: [PATCH v3 2/2] vdpasim: support doorbell mapping

2023-01-28 Thread Jason Wang 
On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike)  wrote:
>
> From: Longpeng 
>
> Support doorbell mapping for vdpasim devices, then we can test the notify
> passthrough feature even if there's no real hardware on hand.
>
> Allocates a dummy page which is used to emulate the notify page of the device,
> all VQs share the same notify register  that initiated to 0x. A  periodic
> work will check whether there're requests need to process ( the value of the
> notify register is 0x or not ).
> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 65 
>  drivers/vdpa/vdpa_sim/vdpa_sim.h |  3 ++
>  2 files changed, 68 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c 
> b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> index b071f0d842fb..4fcfeb6e2fb8 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> @@ -39,6 +39,8 @@ MODULE_PARM_DESC(max_iotlb_entries,
>  #define VDPASIM_QUEUE_ALIGN PAGE_SIZE
>  #define VDPASIM_QUEUE_MAX 256
>  #define VDPASIM_VENDOR_ID 0
> +#define VDPASIM_VRING_POLL_PERIOD 100 /* ms */
> +#define VDPASIM_NOTIFY_DEFVAL 0x
>
>  static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa)
>  {
> @@ -246,6 +248,28 @@ static const struct dma_map_ops vdpasim_dma_ops = {
>  static const struct vdpa_config_ops vdpasim_config_ops;
>  static const struct vdpa_config_ops vdpasim_batch_config_ops;
>
> +static void vdpasim_notify_work(struct work_struct *work)
> +{
> +   struct vdpasim *vdpasim;
> +   u16 *val;
> +
> +   vdpasim = container_of(work, struct vdpasim, notify_work.work);
> +
> +   if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
> +   goto out;
> +
> +   if (!vdpasim->running)
> +   goto out;
> +
> +   val = (u16 *)vdpasim->notify;
> +   if (xchg(val, VDPASIM_NOTIFY_DEFVAL) != VDPASIM_NOTIFY_DEFVAL)
> +   schedule_work(&vdpasim->work);
> +
> +out:
> +   schedule_delayed_work(&vdpasim->notify_work,
> + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD));
> +}
> +
>  struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr,
>const struct vdpa_dev_set_config *config)
>  {
> @@ -287,6 +311,13 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr 
> *dev_attr,
> set_dma_ops(dev, &vdpasim_dma_ops);
> vdpasim->vdpa.mdev = dev_attr->mgmt_dev;
>
> +   INIT_DELAYED_WORK(&vdpasim->notify_work, vdpasim_notify_work);
> +
> +   vdpasim->notify = __get_free_page(GFP_KERNEL | __GFP_ZERO);
> +   if (!vdpasim->notify)
> +   goto err_iommu;

We can simply avoid the advertising notification area in this case.

> +   *(u16 *)vdpasim->notify = VDPASIM_NOTIFY_DEFVAL;

WRITE_ONCE()?

> +
> vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL);
> if (!vdpasim->config)
> goto err_iommu;
> @@ -498,16 +529,21 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa)
>  static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status)
>  {
> struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> +   bool started = vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK;

Do we need to do the check under the vdpasim->lock?

>
> spin_lock(&vdpasim->lock);
> vdpasim->status = status;
> spin_unlock(&vdpasim->lock);
> +   if (!started && (status & VIRTIO_CONFIG_S_DRIVER_OK))
> +   schedule_delayed_work(&vdpasim->notify_work,
> + 
> msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD));
>  }
>
>  static int vdpasim_reset(struct vdpa_device *vdpa)
>  {
> struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
>
> +   cancel_delayed_work_sync(&vdpasim->notify_work);

Do we need to do this after setting running to zero? Otherwise it's racy.

Thanks

> spin_lock(&vdpasim->lock);
> vdpasim->status = 0;
> vdpasim_do_reset(vdpasim);
> @@ -672,11 +708,34 @@ static int vdpasim_dma_unmap(struct vdpa_device *vdpa, 
> unsigned int asid,
> return 0;
>  }
>
> +static pgprot_t vdpasim_get_vq_notification_pgprot(struct vdpa_device *vdpa,
> +  u16 qid, pgprot_t prot)
> +{
> +   /*
> +* We use normal RAM pages to emulate the vq notification area, so
> +* just keep the pgprot as it mmaped.
> +*/
> +   return prot;
> +}
> +
> +static struct vdpa_notification_area
> +vdpasim_get_vq_notification(struct vdpa_device *vdpa, u16 qid)
> +{
> +   struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> +   struct vdpa_notification_area notify;
> +
> +   notify.addr = virt_to_phys((void *)vdpasim->notify);
> +   notify.size = PAGE_SIZE;
> +
> +   return notify;
> +}
> +
>  static void vdpasim_free(struct vdpa_device *vdpa)
>  {
> struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> int i;
>
> +   cancel_delayed_work_sync(&vdpasim->notify_work);
> cancel_work_sync(&vdpa

Re: [PATCH v3 1/2] vdpa: support specify the pgprot of vq notification area

2023-01-28 Thread Jason Wang 
On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike)  wrote:
>
> From: Longpeng 
>
> Adds get_vq_notification_pgprot operation to vdpa_config_ops to support
> specify the pgprot of vq norification area. It's an optional operation,
> the vdpa framework will treat the pgprot of vq notification area as
> noncached as default as usual.

Missing sob.

Other than this.

Acked-by: Jason Wang 

Thanks

> ---
>  drivers/vhost/vdpa.c | 4 +++-
>  include/linux/vdpa.h | 9 +
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 166044642fd5..036fe88425c8 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -1263,7 +1263,9 @@ static vm_fault_t vhost_vdpa_fault(struct vm_fault *vmf)
>
> notify = ops->get_vq_notification(vdpa, index);
>
> -   vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +   vma->vm_page_prot = ops->get_vq_notification_pgprot ?
> +   ops->get_vq_notification_pgprot(vdpa, index, 
> vma->vm_page_prot) :
> +   pgprot_noncached(vma->vm_page_prot);
> if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
> PFN_DOWN(notify.addr), PAGE_SIZE,
> vma->vm_page_prot))
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 6d0f5e4e82c2..07fcf5e6abc8 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -169,6 +169,12 @@ struct vdpa_map_file {
>   * @vdev: vdpa device
>   * @idx: virtqueue index
>   * Returns the notifcation area
> + * @get_vq_notification_pgprot:Get the pgprot of the vq's 
> notification area (optional)
> + * @vdev: vdpa device
> + * @idx: virtqueue index
> + * @prot: original page protection value of the
> + *notification area
> + * Returns pgprot_t: the pgprot of the 
> notification area
>   * @get_vq_irq:Get the irq number of a virtqueue 
> (optional,
>   * but must implemented if require vq irq 
> offloading)
>   * @vdev: vdpa device
> @@ -305,6 +311,9 @@ struct vdpa_config_ops {
>struct netlink_ext_ack *extack);
> struct vdpa_notification_area
> (*get_vq_notification)(struct vdpa_device *vdev, u16 idx);
> +   pgprot_t (*get_vq_notification_pgprot)(struct vdpa_device *vdev,
> +  u16 idx,
> +  pgprot_t prot);
> /* vq irq is not expected to be changed once DRIVER_OK is set */
> int (*get_vq_irq)(struct vdpa_device *vdev, u16 idx);
>
> --
> 2.23.0
>

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

Re: [PATCH] vhost/vdpa: Add MSI translation tables to iommu for software-managed MSI

2023-01-28 Thread Jason Wang 
On Sat, Jan 28, 2023 at 10:25 AM Nanyong Sun  wrote:
>
> From: Rong Wang 
>
> Once enable iommu domain for one device, the MSI
> translation tables have to be there for software-managed MSI.
> Otherwise, platform with software-managed MSI without an
> irq bypass function, can not get a correct memory write event
> from pcie, will not get irqs.
> The solution is to obtain the MSI phy base address from
> iommu reserved region, and set it to iommu MSI cookie,
> then translation tables will be created while request irq.
>
> Signed-off-by: Rong Wang 
> Signed-off-by: Nanyong Sun 
> ---
>  drivers/iommu/iommu.c |  1 +
>  drivers/vhost/vdpa.c  | 53 ---
>  2 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index de91dd88705b..f6c65d5d8e2b 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2623,6 +2623,7 @@ void iommu_get_resv_regions(struct device *dev, struct 
> list_head *list)
> if (ops->get_resv_regions)
> ops->get_resv_regions(dev, list);
>  }
> +EXPORT_SYMBOL_GPL(iommu_get_resv_regions);
>
>  /**
>   * iommu_put_resv_regions - release resered regions
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index ec32f785dfde..31d3e9ed4cfa 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -1103,6 +1103,48 @@ static ssize_t vhost_vdpa_chr_write_iter(struct kiocb 
> *iocb,
> return vhost_chr_write_iter(dev, from);
>  }
>
> +static bool vhost_vdpa_check_sw_msi(struct list_head *dev_resv_regions, 
> phys_addr_t *base)
> +{
> +   struct iommu_resv_region *region;
> +   bool ret = false;
> +
> +   list_for_each_entry(region, dev_resv_regions, list) {
> +   /*
> +* The presence of any 'real' MSI regions should take
> +* precedence over the software-managed one if the
> +* IOMMU driver happens to advertise both types.
> +*/
> +   if (region->type == IOMMU_RESV_MSI) {
> +   ret = false;
> +   break;
> +   }
> +
> +   if (region->type == IOMMU_RESV_SW_MSI) {
> +   *base = region->start;
> +   ret = true;
> +   }
> +   }
> +
> +   return ret;
> +}

Can we unify this with what VFIO had?

> +
> +static int vhost_vdpa_get_msi_cookie(struct iommu_domain *domain, struct 
> device *dma_dev)
> +{
> +   struct list_head dev_resv_regions;
> +   phys_addr_t resv_msi_base = 0;
> +   int ret = 0;
> +
> +   INIT_LIST_HEAD(&dev_resv_regions);
> +   iommu_get_resv_regions(dma_dev, &dev_resv_regions);
> +
> +   if (vhost_vdpa_check_sw_msi(&dev_resv_regions, &resv_msi_base))
> +   ret = iommu_get_msi_cookie(domain, resv_msi_base);
> +
> +   iommu_put_resv_regions(dma_dev, &dev_resv_regions);
> +
> +   return ret;
> +}
> +
>  static int vhost_vdpa_alloc_domain(struct vhost_vdpa *v)
>  {
> struct vdpa_device *vdpa = v->vdpa;
> @@ -1128,11 +1170,16 @@ static int vhost_vdpa_alloc_domain(struct vhost_vdpa 
> *v)
>
> ret = iommu_attach_device(v->domain, dma_dev);
> if (ret)
> -   goto err_attach;
> +   goto err_alloc_domain;
>
> -   return 0;
> +   ret = vhost_vdpa_get_msi_cookie(v->domain, dma_dev);

Do we need to check the overlap mapping and record it in the interval
tree (as what VFIO did)?

Thanks

> +   if (ret)
> +   goto err_attach_device;
>
> -err_attach:
> +   return 0;
> +err_attach_device:
> +   iommu_detach_device(v->domain, dma_dev);
> +err_alloc_domain:
> iommu_domain_free(v->domain);
> return ret;
>  }
> --
> 2.25.1
>

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

Re: [PATCH 2/2] vringh: fetch used_idx from vring at vringh_init_iotlb

2023-01-28 Thread Jason Wang 
On Thu, Jan 19, 2023 at 4:11 PM Eugenio Perez Martin
 wrote:
>
> On Thu, Jan 19, 2023 at 4:20 AM Jason Wang  wrote:
> >
> > On Thu, Jan 19, 2023 at 12:44 AM Eugenio Pérez  wrote:
> > >
> > > Starting from an used_idx different than 0 is needed in use cases like
> > > virtual machine migration.  Not doing so and letting the caller set an
> > > avail idx different than 0 causes destination device to try to use old
> > > buffers that source driver already recover and are not available
> > > anymore.
> > >
> > > While callers like vdpa_sim set avail_idx directly it does not set
> > > used_idx.  Instead of let the caller do the assignment, fetch it from
> > > the guest at initialization like vhost-kernel do.
> > >
> > > To perform the same at vring_kernel_init and vring_user_init is left for
> > > the future.
> > >
> > > Signed-off-by: Eugenio Pérez 
> > > ---
> > >  drivers/vhost/vringh.c | 25 +++--
> > >  1 file changed, 23 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > > index 33eb941fcf15..0eed825197f2 100644
> > > --- a/drivers/vhost/vringh.c
> > > +++ b/drivers/vhost/vringh.c
> > > @@ -1301,6 +1301,17 @@ static inline int putused_iotlb(const struct 
> > > vringh *vrh,
> > > return 0;
> > >  }
> > >
> > > +/**
> > > + * vringh_update_used_idx - fetch used idx from driver's used split vring
> > > + * @vrh: The vring.
> > > + *
> > > + * Returns -errno or 0.
> > > + */
> > > +static inline int vringh_update_used_idx(struct vringh *vrh)
> > > +{
> > > +   return getu16_iotlb(vrh, &vrh->last_used_idx, 
> > > &vrh->vring.used->idx);
> > > +}
> > > +
> > >  /**
> > >   * vringh_init_iotlb - initialize a vringh for a ring with IOTLB.
> > >   * @vrh: the vringh to initialize.
> > > @@ -1319,8 +1330,18 @@ int vringh_init_iotlb(struct vringh *vrh, u64 
> > > features,
> > >   struct vring_avail *avail,
> > >   struct vring_used *used)
> > >  {
> >
> > While at this, I wonder if it's better to have a dedicated parameter
> > for last_avail_idx?
> >
>
> I also had that thought. To directly assign last_avail_idx is not a
> specially elegant API IMO.
>
> Maybe expose a way to fetch used_idx from device vring and pass
> used_idx as parameter too?

If I was not wrong, we can start from last_avail_idx, for used_idx it
is only needed for inflight descriptors which might require other
APIs?

(All the current vDPA user of vringh is doing in order processing)

>
> > > -   return vringh_init_kern(vrh, features, num, weak_barriers,
> > > -   desc, avail, used);
> > > +   int r = vringh_init_kern(vrh, features, num, weak_barriers, desc,
> > > +avail, used);
> > > +
> > > +   if (r != 0)
> > > +   return r;
> > > +
> > > +   /* Consider the ring not initialized */
> > > +   if ((void *)desc == used)
> > > +   return 0;
> >
> > I don't understand when we can get this (actually it should be a bug
> > of the caller).
> >
>
> You can see it in vdpasim_vq_reset.
>
> Note that to consider desc == 0 to be an uninitialized ring is a bug
> IMO. QEMU considers it that way also, but the standard does not forbid
> any ring to be at address 0. Especially if we use vIOMMU.
>
> So I think the best way to know if we can use the vringh is either
> this way, or provide an explicit "initialized" boolean attribute.
> Maybe a new "bool is_initialized(vrh)" is enough, if we don't want to
> add new attributes.

I wonder if we can avoid this in the simulator level instead of the
vringh (anyhow it only exposes a vringh_init_xxx() helper now).

Thanks

>
> Thanks!
>
> > Thanks
> >
> > > +
> > > +   return vringh_update_used_idx(vrh);
> > > +
> > >  }
> > >  EXPORT_SYMBOL(vringh_init_iotlb);
> > >
> > > --
> > > 2.31.1
> > >
> >
>

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

Re: [PATCH 1/2] vdpa_sim: not reset state in vdpasim_queue_ready

2023-01-28 Thread Jason Wang 


在 2023/1/19 17:14, Eugenio Perez Martin 写道:

On Thu, Jan 19, 2023 at 4:16 AM Jason Wang  wrote:

On Thu, Jan 19, 2023 at 12:44 AM Eugenio Pérez  wrote:

vdpasim_queue_ready calls vringh_init_iotlb, which resets split indexes.
But it can be called after setting a ring base with
vdpasim_set_vq_state.

Fix it by stashing them. They're still resetted in vdpasim_vq_reset.

This was discovered and tested live migrating the vdpa_sim_net device.

Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
Signed-off-by: Eugenio Pérez 
---
  drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index cb88891b44a8..8839232a3fcb 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -66,6 +66,7 @@ static void vdpasim_vq_notify(struct vringh *vring)
  static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
  {
 struct vdpasim_virtqueue *vq = &vdpasim->vqs[idx];
+   uint16_t last_avail_idx = vq->vring.last_avail_idx;

 vringh_init_iotlb(&vq->vring, vdpasim->features, vq->num, false,
   (struct vring_desc *)(uintptr_t)vq->desc_addr,
@@ -74,6 +75,7 @@ static void vdpasim_queue_ready(struct vdpasim *vdpasim, 
unsigned int idx)
   (struct vring_used *)
   (uintptr_t)vq->device_addr);

+   vq->vring.last_avail_idx = last_avail_idx;

Does this need to be serialized with the datapath?

E.g in set_vq_state() we do:

spin_lock(&vdpasim->lock);
vrh->last_avail_idx = state->split.avail_index;
spin_unlock(&vdpasim->lock);


vdpasim_queue_ready is called from vdpasim_set_vq_ready, which holds
these locks.

Maybe it's too much indirection and to embed vdpasim_queue_ready in
vdpasim_set_vq_ready would be clearer for the future?



Nope, I miss the caller.

Acked-by: Jason Wang 

Thanks




Thanks!



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

Re: [PATCH 0/4] Fix probe failed when modprobe modules

2023-01-28 Thread Jason Wang 
On Fri, Jan 27, 2023 at 7:12 PM Michael S. Tsirkin  wrote:
>
> On Mon, Nov 28, 2022 at 05:14:44AM -0500, Michael S. Tsirkin wrote:
> > On Mon, Nov 28, 2022 at 10:10:01AM +0800, Li Zetao wrote:
> > > This patchset fixes similar issue, the root cause of the
> > > problem is that the virtqueues are not stopped on error
> > > handling path.
> >
> > I've been thinking about this.
> > Almost all drivers are affected.
> >
> > The reason really is that it used to be the right thing to do:
> > On legacy pci del_vqs writes 0
> > into vq index and this resets the device as a side effect
> > (we actually do this multiple times, what e.g. writes of MSI vector
> >  after the 1st reset do I have no idea).
> >
> > mmio ccw and modern pci don't.
> >
> > Given this has been with us for a while I am inlined to look for
> > a global solution rather than tweaking each driver.
> >
> > Given many drivers are supposed to work on legacy too, we know del_vqs
> > includes a reset for many of them. So I think I see a better way to do
> > this:
> >
> > Add virtio_reset_device_and_del_vqs()
> >
> > and convert all drivers to that.
> >
> > When doing this, we also need to/can fix a related problem (and related
> > to the hardening that Jason Wang was looking into):
> > virtio_reset_device is inherently racy: vq interrupts could
> > be in flight when we do reset. We need to prevent handlers from firing in
> > the window between reset and freeing the irq, so we should first
> > free irqs and only then start changing the state by e.g.
> > device reset.
> >
> >
> > Quite a lot of core work here. Jason are you still looking into
> > hardening?
> >
>
> Li Zetao, Jason, any updates. You guys looking into this?

At least I will continue the work of IRQ hardening. And this work
could be done on top.

Thanks

>
>
> >
> > > Li Zetao (4):
> > >   9p: Fix probe failed when modprobe 9pnet_virtio
> > >   virtio-mem: Fix probe failed when modprobe virtio_mem
> > >   virtio-input: Fix probe failed when modprobe virtio_input
> > >   virtio-blk: Fix probe failed when modprobe virtio_blk
> > >
> > >  drivers/block/virtio_blk.c| 1 +
> > >  drivers/virtio/virtio_input.c | 1 +
> > >  drivers/virtio/virtio_mem.c   | 1 +
> > >  net/9p/trans_virtio.c | 1 +
> > >  4 files changed, 4 insertions(+)
> > >
> > > --
> > > 2.25.1
>

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

Re: [PATCH 3/4] virtio_ring: introduce a per virtqueue waitqueue

2023-01-28 Thread Jason Wang 
On Fri, Jan 27, 2023 at 6:35 PM Michael S. Tsirkin  wrote:
>
> On Fri, Dec 30, 2022 at 11:43:08AM +0800, Jason Wang wrote:
> > On Thu, Dec 29, 2022 at 4:10 PM Michael S. Tsirkin  wrote:
> > >
> > > On Thu, Dec 29, 2022 at 04:04:13PM +0800, Jason Wang wrote:
> > > > On Thu, Dec 29, 2022 at 3:07 PM Michael S. Tsirkin  
> > > > wrote:
> > > > >
> > > > > On Wed, Dec 28, 2022 at 07:53:08PM +0800, Jason Wang wrote:
> > > > > > On Wed, Dec 28, 2022 at 2:34 PM Jason Wang  
> > > > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > > 在 2022/12/27 17:38, Michael S. Tsirkin 写道:
> > > > > > > > On Tue, Dec 27, 2022 at 05:12:58PM +0800, Jason Wang wrote:
> > > > > > > >> 在 2022/12/27 15:33, Michael S. Tsirkin 写道:
> > > > > > > >>> On Tue, Dec 27, 2022 at 12:30:35PM +0800, Jason Wang wrote:
> > > > > > > > But device is still going and will later use the buffers.
> > > > > > > >
> > > > > > > > Same for timeout really.
> > > > > > >  Avoiding infinite wait/poll is one of the goals, another is 
> > > > > > >  to sleep.
> > > > > > >  If we think the timeout is hard, we can start from the wait.
> > > > > > > 
> > > > > > >  Thanks
> > > > > > > >>> If the goal is to avoid disrupting traffic while CVQ is in 
> > > > > > > >>> use,
> > > > > > > >>> that sounds more reasonable. E.g. someone is turning on 
> > > > > > > >>> promisc,
> > > > > > > >>> a spike in CPU usage might be unwelcome.
> > > > > > > >>
> > > > > > > >> Yes, this would be more obvious is UP is used.
> > > > > > > >>
> > > > > > > >>
> > > > > > > >>> things we should be careful to address then:
> > > > > > > >>> 1- debugging. Currently it's easy to see a warning if CPU is 
> > > > > > > >>> stuck
> > > > > > > >>>  in a loop for a while, and we also get a backtrace.
> > > > > > > >>>  E.g. with this - how do we know who has the RTNL?
> > > > > > > >>>  We need to integrate with kernel/watchdog.c for good 
> > > > > > > >>> results
> > > > > > > >>>  and to make sure policy is consistent.
> > > > > > > >>
> > > > > > > >> That's fine, will consider this.
> > > > > >
> > > > > > So after some investigation, it seems the watchdog.c doesn't help. 
> > > > > > The
> > > > > > only export helper is touch_softlockup_watchdog() which tries to 
> > > > > > avoid
> > > > > > triggering the lockups warning for the known slow path.
> > > > >
> > > > > I never said you can just use existing exporting APIs. You'll have to
> > > > > write new ones :)
> > > >
> > > > Ok, I thought you wanted to trigger similar warnings as a watchdog.
> > > >
> > > > Btw, I wonder what kind of logic you want here. If we switch to using
> > > > sleep, there won't be soft lockup anymore. A simple wait + timeout +
> > > > warning seems sufficient?
> > > >
> > > > Thanks
> > >
> > > I'd like to avoid need to teach users new APIs. So watchdog setup to apply
> > > to this driver. The warning can be different.
> >
> > Right, so it looks to me the only possible setup is the
> > watchdog_thres. I plan to trigger the warning every watchdog_thres * 2
> > second (as softlockup did).
> >
> > And I think it would still make sense to fail, we can start with a
> > very long timeout like 1 minutes and break the device. Does this make
> > sense?
> >
> > Thanks
>
> I'd say we need to make this manageable then.

Did you mean something like sysfs or module parameters?

> Can't we do it normally
> e.g. react to an interrupt to return to userspace?

I didn't get the meaning of this. Sorry.

Thanks

>
>
>
> > >
> > >
> > > > >
> > > > > > And before the patch, we end up with a real infinite loop which 
> > > > > > could
> > > > > > be caught by RCU stall detector which is not the case of the sleep.
> > > > > > What we can do is probably do a periodic netdev_err().
> > > > > >
> > > > > > Thanks
> > > > >
> > > > > Only with a bad device.
> > > > >
> > > > > > > >>
> > > > > > > >>
> > > > > > > >>> 2- overhead. In a very common scenario when device is in 
> > > > > > > >>> hypervisor,
> > > > > > > >>>  programming timers etc has a very high overhead, at 
> > > > > > > >>> bootup
> > > > > > > >>>  lots of CVQ commands are run and slowing boot down is 
> > > > > > > >>> not nice.
> > > > > > > >>>  let's poll for a bit before waiting?
> > > > > > > >>
> > > > > > > >> Then we go back to the question of choosing a good timeout for 
> > > > > > > >> poll. And
> > > > > > > >> poll seems problematic in the case of UP, scheduler might not 
> > > > > > > >> have the
> > > > > > > >> chance to run.
> > > > > > > > Poll just a bit :) Seriously I don't know, but at least check 
> > > > > > > > once
> > > > > > > > after kick.
> > > > > > >
> > > > > > >
> > > > > > > I think it is what the current code did where the condition will 
> > > > > > > be
> > > > > > > check before trying to sleep in the wait_event().
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > >>> 3- suprise removal. need to wake up thread in some way. what 
> > > > > > > >>> about
>

Re: [PATCH] vdpa_sim: get rid of DMA ops

2023-01-28 Thread Jason Wang 
On Fri, Jan 27, 2023 at 6:29 PM Michael S. Tsirkin  wrote:
>
> On Mon, Dec 26, 2022 at 12:12:42PM +0800, Jason Wang wrote:
> > > >@@ -682,6 +553,11 @@ static int vdpasim_dma_unmap(struct vdpa_device 
> > > >*vdpa, unsigned int asid,
> > > >   if (asid >= vdpasim->dev_attr.nas)
> > > >   return -EINVAL;
> > > >
> > > >+  if (vdpasim->iommu_pt[asid]) {
> > >
> > > We are in the vdpasim_dma_unmap, so if vdpasim->iommu_pt[asid] is true,
> > > should be better to return an error, since this case should not happen?
> >
> > So it's a question of how to behave when unmap is called without a
> > map. I think we can leave the code as is or if we wish, it needs a
> > separate patch.
> >
> > (We didn't error this previously anyhow).
> >
> > Thanks
>
> OK I picked as is. Do we want WARN_ON maybe?

This could be triggered by the userspace, so I'm not sure it's worth it.

Thanks

>
> --
> MST
>

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

CISTI'2023 - Doctoral Symposium |Aveiro, Portugal

2023-01-28 Thread CISTI-2023 
* Published in IEEE Xplore

* Google Scholar H5-Index = 22




--    Doctoral Symposium of CISTI'2023 
--   
CISTI'2023 - 18th Iberian Conference on Information Systems and Technologies

20 - 23 of June 2023, University of Aveiro, Aveiro, Portugal

http://cisti.eu/ 


--   --   
--   --   



The purpose of CISTI'2022’s Doctoral Symposium is to provide graduate students 
a setting where they can, informally, expose and discuss their work, collecting 
valuable expert opinions and sharing new ideas, methods and applications. The 
Doctoral Symposium is an excellent opportunity for PhD students to present and 
discuss their work in a Workshop format. Each presentation will be evaluated by 
a panel composed by at least three Information Systems and Technologies experts.



Contributions Submission

The Doctoral Symposium is opened to PhD students whose research area includes 
the themes proposed for this Conference. Submissions must include an extended 
abstract (maximum 4 pages), following the Conference style guide 
.
 All selected contributions will be published with the Conference Proceedings 
in electronic format with ISBN. These contributions will be available in the 
IEEE Xplore 

 Digital Library and will be sent for indexing in ISI, Scopus, EI-Compendex, 
INSPEC and Google Scholar.

Submissions must include the field, the PhD institution and the number of 
months devoted to the development of the work. Additionally, they should 
include in a clear and succinct manner:

•The problem approached and its significance or relevance
•The research objectives and related investigation topics
•A brief display of what is already known
•A proposed solution methodology for the problem
•Expected results




Important Dates

Paper submission: February 19, 2023

Notification of acceptance: March 26, 2023

Submission of accepted papers: April 9, 2023

Payment of registration, to ensure the inclusion of an accepted paper in the 
conference proceedings: April 9, 2023



Organizing Committee


Álvaro Rocha, ISEG, Universidade de Lisboa

Francisco García-Peñalvo, Universidad de Salamanca



Scientific Committee

Francisco García-Peñalvo, Universidad de Salamanca (Presidente)

Augusto Sousa, FEUP, Universidade do Porto

Adolfo Lozano Tello, Universidad de Extremadura

Álvaro Rocha, ISEG, Universidade de Lisboa

Ana Amélia Carvalho, Universidade de Coimbra

António Coelho, FEUP, Universidade do Porto

Antonio Garcia Loureiro, Universidade de Santiago de Compostela

António Palma do Reis, ISEG, Universidade de Lisboa

Arnaldo Martins, Universidade de Aveiro

Borja Bordel, Universidad Politécnica de Madrid

Carlos Costa, ISEG, Universidade de Lisboa

Carlos Ferrás Sexto, Universidad de Santiago de Compostela

Carlos Montenegro, Universidad Distrital Francisco José de Caldas

Cesar Collazos, Universidad del Cauca

David Fonseca, La Salle, Universitat Ramon Llull

Fernando Moreira, Universidade Portucalense

Gonçalo Paiva Dias, Universidade de Aveiro

Jeimy Cano, Universidad de los Andes

Jezreel Mejia, CIMAT

João Manuel R.S. Tavares, FEUP, Universidade do Porto

João Pascoal Faria, FEUP, Universidade do Porto~

João Paulo Costa, Universidade de Coimbra

José Machado, Universidade do Minho

Luis Camarinha-Matos, Universidade NOVA de Lisboa

Manuel Tupia, Pontifica Universidad Católica del Perú

Marcelo Marciszack, Universidad Tecnológica Nacional

Marco Painho, Nova Information Management School, Universidade Nova de Lisboa

María J Lado, Universidade de Vigo

Maria José Sousa, ISCTE - Instituto Universitário de Lisboa

Mário Piattini, Universidad de Castilla-La Mancha

Maristela Holanda, Universidade de Brasília

Mercedes Ruiz, Universidad de Cádiz

Miguel Casquilho, Universidade de Lisboa

Miguel de Castro Neto, NOVA IMS

Miguel Ramón Gonzalez Castro, Centro Tecnológico Aimen

Mirna Muñoz, Centro de Investigación en Matemáticas A.C.- Unidad Zacatecas

Nelson Rocha, Universidade de Aveiro

Óscar Mealha, Universidade de Aveiro

Paulo Pinto, FC, Universidade Nova de Lisboa

Ramiro Gonçalves, Universidade de Trás-os-Montes e Alto Douro

Rui Cruz, IST, Universidade de Lisboa

Victor Hugo Medina Garcia, Universidad Distrital Francisco José de Caldas

Vítor Santos, NOVA IMS







Website of CISTI'2023: http://cisti.eu/ 



--
This email has been checked for viruses by AVG antivirus software.
www.avg.com_