Re: [RFC PATCH v1 11/25] hw/xen: Pass grant ref to gnttab unmap operation

2023-03-07 Thread Paul Durrant

On 02/03/2023 15:34, David Woodhouse wrote:

From: David Woodhouse 

The previous commit introduced redirectable gnttab operations fairly
much like-for-like, with the exception of the extra arguments to the
->open() call which were always NULL/0 anyway.

This *changes* the arguments to the ->unmap() operation to include the
original ref# that was mapped. Under real Xen it isn't necessary; all we
need to do from QEMU is munmap(), then the kernel will release the grant,
and Xen does the tracking/refcounting for the guest.

When we have emulated grant tables though, we need to do all that for
ourselves. So let's have the back ends keep track of what they mapped
and pass it in to the ->unmap() method for us.

Signed-off-by: David Woodhouse 
---
  hw/9pfs/xen-9p-backend.c|  7 ---
  hw/block/dataplane/xen-block.c  |  1 +
  hw/char/xen_console.c   |  2 +-
  hw/net/xen_nic.c| 13 -
  hw/usb/xen-usb.c| 21 -
  hw/xen/xen-bus.c|  4 ++--
  hw/xen/xen-legacy-backend.c |  4 ++--
  hw/xen/xen-operations.c |  9 -
  include/hw/xen/xen-bus.h|  2 +-
  include/hw/xen/xen-legacy-backend.h |  6 +++---
  include/hw/xen/xen_backend_ops.h|  7 ---
  11 files changed, 50 insertions(+), 26 deletions(-)



Reviewed-by: Paul Durrant 




[RFC PATCH v1 11/25] hw/xen: Pass grant ref to gnttab unmap operation

2023-03-02 Thread David Woodhouse
From: David Woodhouse 

The previous commit introduced redirectable gnttab operations fairly
much like-for-like, with the exception of the extra arguments to the
->open() call which were always NULL/0 anyway.

This *changes* the arguments to the ->unmap() operation to include the
original ref# that was mapped. Under real Xen it isn't necessary; all we
need to do from QEMU is munmap(), then the kernel will release the grant,
and Xen does the tracking/refcounting for the guest.

When we have emulated grant tables though, we need to do all that for
ourselves. So let's have the back ends keep track of what they mapped
and pass it in to the ->unmap() method for us.

Signed-off-by: David Woodhouse 
---
 hw/9pfs/xen-9p-backend.c|  7 ---
 hw/block/dataplane/xen-block.c  |  1 +
 hw/char/xen_console.c   |  2 +-
 hw/net/xen_nic.c| 13 -
 hw/usb/xen-usb.c| 21 -
 hw/xen/xen-bus.c|  4 ++--
 hw/xen/xen-legacy-backend.c |  4 ++--
 hw/xen/xen-operations.c |  9 -
 include/hw/xen/xen-bus.h|  2 +-
 include/hw/xen/xen-legacy-backend.h |  6 +++---
 include/hw/xen/xen_backend_ops.h|  7 ---
 11 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 864bdaf952..d8bb0e847c 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -359,12 +359,13 @@ static int xen_9pfs_free(struct XenLegacyDevice *xendev)
 if (xen_9pdev->rings[i].data != NULL) {
 xen_be_unmap_grant_refs(&xen_9pdev->xendev,
 xen_9pdev->rings[i].data,
+xen_9pdev->rings[i].intf->ref,
 (1 << xen_9pdev->rings[i].ring_order));
 }
 if (xen_9pdev->rings[i].intf != NULL) {
-xen_be_unmap_grant_refs(&xen_9pdev->xendev,
-xen_9pdev->rings[i].intf,
-1);
+xen_be_unmap_grant_ref(&xen_9pdev->xendev,
+   xen_9pdev->rings[i].intf,
+   xen_9pdev->rings[i].ref);
 }
 if (xen_9pdev->rings[i].bh != NULL) {
 qemu_bh_delete(xen_9pdev->rings[i].bh);
diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index 2785b9e849..e55b713002 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -705,6 +705,7 @@ void xen_block_dataplane_stop(XenBlockDataPlane *dataplane)
 Error *local_err = NULL;
 
 xen_device_unmap_grant_refs(xendev, dataplane->sring,
+dataplane->ring_ref,
 dataplane->nr_ring_ref, &local_err);
 dataplane->sring = NULL;
 
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 63153dfde4..19ad6c946a 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -271,7 +271,7 @@ static void con_disconnect(struct XenLegacyDevice *xendev)
 if (!xendev->dev) {
 xenforeignmemory_unmap(xen_fmem, con->sring, 1);
 } else {
-xen_be_unmap_grant_ref(xendev, con->sring);
+xen_be_unmap_grant_ref(xendev, con->sring, con->ring_ref);
 }
 con->sring = NULL;
 }
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 7d92c2d022..166d03787d 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -181,7 +181,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
 qemu_send_packet(qemu_get_queue(netdev->nic),
  page + txreq.offset, txreq.size);
 }
-xen_be_unmap_grant_ref(&netdev->xendev, page);
+xen_be_unmap_grant_ref(&netdev->xendev, page, txreq.gref);
 net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
 }
 if (!netdev->tx_work) {
@@ -261,7 +261,7 @@ static ssize_t net_rx_packet(NetClientState *nc, const 
uint8_t *buf, size_t size
 return -1;
 }
 memcpy(page + NET_IP_ALIGN, buf, size);
-xen_be_unmap_grant_ref(&netdev->xendev, page);
+xen_be_unmap_grant_ref(&netdev->xendev, page, rxreq.gref);
 net_rx_response(netdev, &rxreq, NETIF_RSP_OKAY, NET_IP_ALIGN, size, 0);
 
 return size;
@@ -343,7 +343,8 @@ static int net_connect(struct XenLegacyDevice *xendev)
netdev->rx_ring_ref,
PROT_READ | PROT_WRITE);
 if (!netdev->rxs) {
-xen_be_unmap_grant_ref(&netdev->xendev, netdev->txs);
+xen_be_unmap_grant_ref(&netdev->xendev, netdev->txs,
+   netdev->tx_ring_ref);
 netdev->txs = NULL;
 return -1;
 }
@@ -368,11 +369,13 @@ static void net_disconnect(struct XenLegacyDevice *xendev)
 xen_pv_unbind_evtchn(&netdev->xendev);
 
 if (ne