[Lguest] [PATCH 2/3] lguest: Simplify device descriptor helpers

Mark McLoughlin markmc at redhat.com
Fri Jun 13 23:04:59 EST 2008


Confusingly, we use device_config() in add_virtqueue() to
obtain a pointer to the next lguest_vqconfig pointer in
the device descriptor, under the assumption that we haven't
yet set up the feature bitmasks or device config.

There's enough going on here that one might get horribly
confused by this for a second.

Simplify the code by adding helpers similar to lg_vq(),
lg_features() and lg_config() functions from the kernel's
guest device code.

That way it's a bit more clear which config is which.

Signed-off-by: Mark McLoughlin <markmc at redhat.com>
---
 Documentation/lguest/lguest.c |   46 +++++++++++++++++++++-------------------
 1 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c
index 4eddaac..40bec7e 100644
--- a/Documentation/lguest/lguest.c
+++ b/Documentation/lguest/lguest.c
@@ -199,13 +199,6 @@ static void *_convert(struct iovec *iov, size_t size, size_t align,
 #define le32_to_cpu(v32) (v32)
 #define le64_to_cpu(v64) (v64)
 
-/* The device virtqueue descriptors are followed by feature bitmasks. */
-static u8 *get_feature_bits(struct device *dev)
-{
-	return (u8 *)(dev->desc + 1)
-		+ dev->desc->num_vq * sizeof(struct lguest_vqconfig);
-}
-
 /*L:100 The Launcher code itself takes us out into userspace, that scary place
  * where pointers run wild and free!  Unfortunately, like most userspace
  * programs, it's quite boring (which is why everyone likes to hack on the
@@ -935,6 +928,8 @@ static void enable_fd(int fd, struct virtqueue *vq)
 	write(waker_fd, &vq->dev->fd, sizeof(vq->dev->fd));
 }
 
+static u8 *device_features(const struct device *dev);
+
 /* When the Guest tells us they updated the status field, we handle it. */
 static void update_device_status(struct device *dev)
 {
@@ -945,7 +940,7 @@ static void update_device_status(struct device *dev)
 		verbose("Resetting device %s\n", dev->name);
 
 		/* Clear any features they've acked. */
-		memset(get_feature_bits(dev) + dev->desc->feature_len, 0,
+		memset(device_features(dev) + dev->desc->feature_len, 0,
 		       dev->desc->feature_len);
 
 		/* Zero out the virtqueues. */
@@ -961,10 +956,10 @@ static void update_device_status(struct device *dev)
 
 		verbose("Device %s OK: offered", dev->name);
 		for (i = 0; i < dev->desc->feature_len; i++)
-			verbose(" %08x", get_feature_bits(dev)[i]);
+			verbose(" %08x", device_features(dev)[i]);
 		verbose(", accepted");
 		for (i = 0; i < dev->desc->feature_len; i++)
-			verbose(" %08x", get_feature_bits(dev)
+			verbose(" %08x", device_features(dev)
 				[dev->desc->feature_len+i]);
 
 		if (dev->ready)
@@ -1064,13 +1059,23 @@ static void handle_input(int fd)
 
 /* The layout of the device page is a "struct lguest_device_desc" followed by a
  * number of virtqueue descriptors, then two sets of feature bits, then an
- * array of configuration bytes.  This routine returns the configuration
- * pointer. */
+ * array of configuration bytes.  This routine returns a pointer to the
+ * virtqueue descriptors. */
+static struct lguest_vqconfig *device_vq(const struct device *dev)
+{
+	return (void *)(dev->desc + 1);
+}
+
+/* The device virtqueue descriptors are followed by feature bitmasks. */
+static u8 *device_features(const struct device *dev)
+{
+	return (void *)(device_vq(dev) + dev->desc->num_vq);
+}
+
+/* The device configuration follows the feature bitmasks */
 static u8 *device_config(const struct device *dev)
 {
-	return (void *)(dev->desc + 1)
-		+ dev->desc->num_vq * sizeof(struct lguest_vqconfig)
-		+ dev->desc->feature_len * 2;
+	return device_features(dev) + dev->desc->feature_len * 2;
 }
 
 /* This routine allocates a new "struct lguest_device_desc" from descriptor
@@ -1124,12 +1129,9 @@ static void add_virtqueue(struct device *dev, unsigned int num_descs,
 	/* Initialize the vring. */
 	vring_init(&vq->vring, num_descs, p, getpagesize());
 
-	/* Append virtqueue to this device's descriptor.  We use
-	 * device_config() to get the end of the device's current virtqueues;
-	 * we check that we haven't added any config or feature information
-	 * yet, otherwise we'd be overwriting them. */
-	assert(dev->desc->config_len == 0 && dev->desc->feature_len == 0);
-	memcpy(device_config(dev), &vq->config, sizeof(vq->config));
+	/* Append virtqueue to this device's descriptor. */
+	memcpy(device_vq(dev) + dev->desc->num_vq,
+	       &vq->config, sizeof(vq->config));
 	dev->desc->num_vq++;
 
 	verbose("Virtqueue page %#lx\n", to_guest_phys(p));
@@ -1153,7 +1155,7 @@ static void add_virtqueue(struct device *dev, unsigned int num_descs,
  * second half is for the Guest to accept features. */
 static void add_feature(struct device *dev, unsigned bit)
 {
-	u8 *features = get_feature_bits(dev);
+	u8 *features = device_features(dev);
 
 	/* We can't extend the feature bits once we've added config bytes */
 	if (dev->desc->feature_len <= bit / CHAR_BIT) {
-- 
1.5.4.1




More information about the Lguest mailing list