[Lguest] [PATCH 4/8] lguest: update commentry
Paul E. McKenney
paulmck at linux.vnet.ibm.com
Fri Jul 24 09:00:11 EST 2009
On Thu, Jul 23, 2009 at 11:16:12PM +0930, Rusty Russell wrote:
>
> Every so often, after code shuffles, I need to go through and unbitrot
> the Lguest Journey (see drivers/lguest/README). Since we now use RCU in
> a simple form in one place I took the opportunity to expand that explanation.
Looks good!!!
Some comments inline. Will you also be commenting the read side in
send_notify_to_eventfd()?
Thanx, Paul
> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
> Cc: Ingo Molnar <mingo at redhat.com>
> Cc: Paul McKenney <paulmck at linux.vnet.ibm.com>
> ---
> Documentation/lguest/lguest.c | 184 +++++++++++++++++++++++++++---------
> arch/x86/include/asm/lguest_hcall.h | 8 -
> arch/x86/lguest/boot.c | 99 +++++++++++++++----
> arch/x86/lguest/i386_head.S | 2
> drivers/lguest/core.c | 7 +
> drivers/lguest/hypercalls.c | 6 -
> drivers/lguest/lguest_device.c | 11 +-
> drivers/lguest/lguest_user.c | 47 ++++++++-
> drivers/lguest/page_tables.c | 84 ++++++++++++----
> drivers/lguest/x86/core.c | 2
> drivers/lguest/x86/switcher_32.S | 6 -
> 11 files changed, 353 insertions(+), 103 deletions(-)
>
> diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c
> --- a/Documentation/lguest/lguest.c
> +++ b/Documentation/lguest/lguest.c
> @@ -49,7 +49,7 @@
> #include "linux/virtio_ring.h"
> #include "asm/bootparam.h"
> /*L:110
> - * We can ignore the 39 include files we need for this program, but I do want
> + * We can ignore the 42 include files we need for this program, but I do want
> * to draw attention to the use of kernel-style types.
> *
> * As Linus said, "C is a Spartan language, and so should your naming be." I
> @@ -305,6 +305,11 @@ static void *map_zeroed_pages(unsigned i
> PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, fd, 0);
> if (addr == MAP_FAILED)
> err(1, "Mmaping %u pages of /dev/zero", num);
> +
> + /*
> + * One neat mmap feature is that you can close the fd, and it
> + * stays mapped.
> + */
> close(fd);
>
> return addr;
> @@ -557,7 +562,7 @@ static void tell_kernel(unsigned long st
> }
> /*:*/
>
> -/*
> +/*L:200
> * Device Handling.
> *
> * When the Guest gives us a buffer, it sends an array of addresses and sizes.
> @@ -608,7 +613,10 @@ static unsigned next_desc(struct vring_d
> return next;
> }
>
> -/* This actually sends the interrupt for this virtqueue */
> +/*
> + * This actually sends the interrupt for this virtqueue, if we've used a
> + * buffer.
> + */
> static void trigger_irq(struct virtqueue *vq)
> {
> unsigned long buf[] = { LHREQ_IRQ, vq->config.irq };
> @@ -629,12 +637,12 @@ static void trigger_irq(struct virtqueue
> }
>
> /*
> - * This looks in the virtqueue and for the first available buffer, and converts
> + * This looks in the virtqueue for the first available buffer, and converts
> * it to an iovec for convenient access. Since descriptors consist of some
> * number of output then some number of input descriptors, it's actually two
> * iovecs, but we pack them into one and note how many of each there were.
> *
> - * This function returns the descriptor number found.
> + * This function waits if necessary, and returns the descriptor number found.
> */
> static unsigned wait_for_vq_desc(struct virtqueue *vq,
> struct iovec iov[],
> @@ -644,10 +652,14 @@ static unsigned wait_for_vq_desc(struct
> struct vring_desc *desc;
> u16 last_avail = lg_last_avail(vq);
>
> + /* There's nothing available? */
> while (last_avail == vq->vring.avail->idx) {
> u64 event;
>
> - /* OK, tell Guest about progress up to now. */
> + /*
> + * Since we're about to sleep, now is a good time to tell the
> + * Guest about what we've used up to now.
> + */
> trigger_irq(vq);
>
> /* OK, now we need to know about added descriptors. */
> @@ -734,8 +746,9 @@ static unsigned wait_for_vq_desc(struct
> }
>
> /*
> - * After we've used one of their buffers, we tell them about it. We'll then
> - * want to send them an interrupt, using trigger_irq().
> + * After we've used one of their buffers, we tell the Guest about it. Sometime
> + * later we'll want to send them an interrupt using trigger_irq(); note that
> + * wait_for_vq_desc() does that for us if it has to wait.
> */
> static void add_used(struct virtqueue *vq, unsigned int head, int len)
> {
> @@ -782,12 +795,12 @@ static void console_input(struct virtque
> struct console_abort *abort = vq->dev->priv;
> struct iovec iov[vq->vring.num];
>
> - /* Make sure there's a descriptor waiting. */
> + /* Make sure there's a descriptor available. */
> head = wait_for_vq_desc(vq, iov, &out_num, &in_num);
> if (out_num)
> errx(1, "Output buffers in console in queue?");
>
> - /* Read it in. */
> + /* Read into it. This is where we usually wait. */
> len = readv(STDIN_FILENO, iov, in_num);
> if (len <= 0) {
> /* Ran out of input? */
> @@ -800,6 +813,7 @@ static void console_input(struct virtque
> pause();
> }
>
> + /* Tell the Guest we used a buffer. */
> add_used_and_trigger(vq, head, len);
>
> /*
> @@ -834,15 +848,23 @@ static void console_output(struct virtqu
> unsigned int head, out, in;
> struct iovec iov[vq->vring.num];
>
> + /* We usually wait in here, for the Guest to give us something. */
> head = wait_for_vq_desc(vq, iov, &out, &in);
> if (in)
> errx(1, "Input buffers in console output queue?");
> +
> + /* writev can return a partial write, so we loop here. */
> while (!iov_empty(iov, out)) {
> int len = writev(STDOUT_FILENO, iov, out);
> if (len <= 0)
> err(1, "Write to stdout gave %i", len);
> iov_consume(iov, out, len);
> }
> +
> + /*
> + * We're finished with that buffer: if we're going to sleep,
> + * wait_for_vq_desc() will prod the Guest with an interrupt.
> + */
> add_used(vq, head, 0);
> }
>
> @@ -862,15 +884,30 @@ static void net_output(struct virtqueue
> unsigned int head, out, in;
> struct iovec iov[vq->vring.num];
>
> + /* We usually wait in here for the Guest to give us a packet. */
> head = wait_for_vq_desc(vq, iov, &out, &in);
> if (in)
> errx(1, "Input buffers in net output queue?");
> + /*
> + * Send the whole thing through to /dev/net/tun. It expects the exact
> + * same format: what a coincidence!
> + */
> if (writev(net_info->tunfd, iov, out) < 0)
> errx(1, "Write to tun failed?");
> +
> + /*
> + * Done with that one; wait_for_vq_desc() will send the interrupt if
> + * all packets are processed.
> + */
> add_used(vq, head, 0);
> }
>
> -/* Will reading from this file descriptor block? */
> +/*
> + * Handling network input is a bit trickier, because I've tried to optimize it.
> + *
> + * First we have a helper routine which tells is if from this file descriptor
> + * (ie. the /dev/net/tun device) will block:
> + */
> static bool will_block(int fd)
> {
> fd_set fdset;
> @@ -880,7 +917,11 @@ static bool will_block(int fd)
> return select(fd+1, &fdset, NULL, NULL, &zero) != 1;
> }
>
> -/* This handles packets coming in from the tun device to our Guest. */
> +/*
> + * This handles packets coming in from the tun device to our Guest. Like all
> + * service routines, it gets called again as soon as it returns, so you don't
> + * see a while(1) loop here.
> + */
> static void net_input(struct virtqueue *vq)
> {
> int len;
> @@ -888,21 +929,38 @@ static void net_input(struct virtqueue *
> struct iovec iov[vq->vring.num];
> struct net_info *net_info = vq->dev->priv;
>
> + /*
> + * Get a descriptor to write an incoming packet into. This will also
> + * send an interrupt if they're out of descriptors.
> + */
> head = wait_for_vq_desc(vq, iov, &out, &in);
> if (out)
> errx(1, "Output buffers in net input queue?");
>
> - /* Deliver interrupt now, since we're about to sleep. */
> + /*
> + * If it looks like we'll block reading from the tun device, send them
> + * an interrupt.
> + */
> if (vq->pending_used && will_block(net_info->tunfd))
> trigger_irq(vq);
>
> + /*
> + * Read in the packet. This is where we normally wait (when there's no
> + * incoming network traffic).
> + */
> len = readv(net_info->tunfd, iov, in);
> if (len <= 0)
> err(1, "Failed to read from tun.");
> +
> + /*
> + * Mark that packet buffer as used, but don't interrupt here. We want
> + * to wait until we've done as much work as we can.
> + */
> add_used(vq, head, len);
> }
> +/*:*/
>
> -/* This is the helper to create threads. */
> +/* This is the helper to create threads: run the service routine in a loop. */
> static int do_thread(void *_vq)
> {
> struct virtqueue *vq = _vq;
> @@ -950,11 +1008,14 @@ static void reset_device(struct device *
> signal(SIGCHLD, (void *)kill_launcher);
> }
>
> +/*L:216
> + * This actually creates the thread which services the virtqueue for a device.
> + */
> static void create_thread(struct virtqueue *vq)
> {
> /*
> - * Create stack for thread and run it. Since the stack grows upwards,
> - * we point the stack pointer to the end of this region.
> + * Create stack for thread. Since the stack grows upwards, we point
> + * the stack pointer to the end of this region.
> */
> char *stack = malloc(32768);
> unsigned long args[] = { LHREQ_EVENTFD,
> @@ -966,17 +1027,22 @@ static void create_thread(struct virtque
> err(1, "Creating eventfd");
> args[2] = vq->eventfd;
>
> - /* Attach an eventfd to this virtqueue: it will go off
> - * when the Guest does an LHCALL_NOTIFY for this vq. */
> + /*
> + * Attach an eventfd to this virtqueue: it will go off when the Guest
> + * does an LHCALL_NOTIFY for this vq.
> + */
> if (write(lguest_fd, &args, sizeof(args)) != 0)
> err(1, "Attaching eventfd");
>
> - /* CLONE_VM: because it has to access the Guest memory, and
> - * SIGCHLD so we get a signal if it dies. */
> + /*
> + * CLONE_VM: because it has to access the Guest memory, and SIGCHLD so
> + * we get a signal if it dies.
> + */
> vq->thread = clone(do_thread, stack + 32768, CLONE_VM | SIGCHLD, vq);
> if (vq->thread == (pid_t)-1)
> err(1, "Creating clone");
> - /* We close our local copy, now the child has it. */
> +
> + /* We close our local copy now the child has it. */
> close(vq->eventfd);
> }
>
> @@ -1028,7 +1094,10 @@ static void update_device_status(struct
> }
> }
>
> -/* This is the generic routine we call when the Guest uses LHCALL_NOTIFY. */
> +/*L:215
> + * This is the generic routine we call when the Guest uses LHCALL_NOTIFY. In
> + * particular, it's used to notify us of device status changes during boot.
> + */
> static void handle_output(unsigned long addr)
> {
> struct device *i;
> @@ -1037,18 +1106,32 @@ static void handle_output(unsigned long
> for (i = devices.dev; i; i = i->next) {
> struct virtqueue *vq;
>
> - /* Notifications to device descriptors update device status. */
> + /*
> + * Notifications to device descriptors mean they updated the
> + * device status.
> + */
> if (from_guest_phys(addr) == i->desc) {
> update_device_status(i);
> return;
> }
>
> - /* Devices *can* be used before status is set to DRIVER_OK. */
> + /*
> + * Devices *can* be used before status is set to DRIVER_OK.
> + * The original plan was that they would never do this: they
> + * would always finish setting up their status bits before
> + * actually touching the virtqueues. In practice, we allowed
> + * them to, and they do (eg. the disk probes for partition
> + * tables as part of initialization).
> + *
> + * If we see this, we start the device: once it's running, we
> + * expect the device to catch all the notifications.
> + */
> for (vq = i->vq; vq; vq = vq->next) {
> if (addr != vq->config.pfn*getpagesize())
> continue;
> if (i->running)
> errx(1, "Notification on running %s", i->name);
> + /* This just calls create_thread() for each virtqueue */
> start_device(i);
> return;
> }
> @@ -1132,6 +1215,11 @@ static void add_virtqueue(struct device
> vq->next = NULL;
> vq->last_avail_idx = 0;
> vq->dev = dev;
> +
> + /*
> + * This is the routine the service thread will run, and its Process ID
> + * once it's running.
> + */
> vq->service = service;
> vq->thread = (pid_t)-1;
>
> @@ -1202,7 +1290,8 @@ static void set_config(struct device *de
>
> /*
> * This routine does all the creation and setup of a new device, including
> - * calling new_dev_desc() to allocate the descriptor and device memory.
> + * calling new_dev_desc() to allocate the descriptor and device memory. We
> + * don't actually start the service threads until later.
> *
> * See what I mean about userspace being boring?
> */
> @@ -1478,19 +1567,7 @@ static void setup_tun_net(char *arg)
> verbose("device %u: tun %s: %s\n",
> devices.device_num, tapif, arg);
> }
> -
> -/*
> - * Our block (disk) device should be really simple: the Guest asks for a block
> - * number and we read or write that position in the file. Unfortunately, that
> - * was amazingly slow: the Guest waits until the read is finished before
> - * running anything else, even if it could have been doing useful work.
> - *
> - * We could use async I/O, except it's reputed to suck so hard that characters
> - * actually go missing from your code when you try to use it.
> - *
> - * So this was one reason why lguest now does all virtqueue servicing in
> - * separate threads: it's more efficient and more like a real device.
> - */
> +/*:*/
>
> /* This hangs off device->priv. */
> struct vblk_info
> @@ -1512,8 +1589,16 @@ struct vblk_info
> /*L:210
> * The Disk
> *
> - * Remember that the block device is handled by a separate I/O thread. We head
> - * straight into the core of that thread here:
> + * The disk only has one virtqueue, so it only has one thread. It is really
> + * simple: the Guest asks for a block number and we read or write that position
> + * in the file.
> + *
> + * Before we serviced each virtqueue in a separate thread, that was unacceptably
> + * slow: the Guest waits until the read is finished before running anything
> + * else, even if it could have been doing useful work.
> + *
> + * We could have used async I/O, except it's reputed to suck so hard that
> + * characters actually go missing from your code when you try to use it.
> */
> static void blk_request(struct virtqueue *vq)
> {
> @@ -1525,7 +1610,10 @@ static void blk_request(struct virtqueue
> struct iovec iov[vq->vring.num];
> off64_t off;
>
> - /* Get the next request. */
> + /*
> + * Get the next request, where we normally wait. It triggers the
> + * interrupt to acknowledge previously serviced requests (if any).
> + */
> head = wait_for_vq_desc(vq, iov, &out_num, &in_num);
>
> /*
> @@ -1539,6 +1627,10 @@ static void blk_request(struct virtqueue
>
> out = convert(&iov[0], struct virtio_blk_outhdr);
> in = convert(&iov[out_num+in_num-1], u8);
> + /*
> + * For historical reasons, block operations are expressed in 512 byte
> + * "sectors".
> + */
> off = out->sector * 512;
>
> /*
> @@ -1614,6 +1706,7 @@ static void blk_request(struct virtqueue
> if (out->type & VIRTIO_BLK_T_BARRIER)
> fdatasync(vblk->fd);
>
> + /* Finished that request. */
> add_used(vq, head, wlen);
> }
>
> @@ -1682,9 +1775,8 @@ static void rng_input(struct virtqueue *
> errx(1, "Output buffers in rng?");
>
> /*
> - * This is why we convert to iovecs: the readv() call uses them, and so
> - * it reads straight into the Guest's buffer. We loop to make sure we
> - * fill it.
> + * Just like the console write, we loop to cover the whole iovec.
> + * In this case, short reads actually happen quite a bit.
> */
> while (!iov_empty(iov, in_num)) {
> len = readv(rng_info->rfd, iov, in_num);
> @@ -1818,7 +1910,9 @@ int main(int argc, char *argv[])
> devices.lastdev = NULL;
> devices.next_irq = 1;
>
> + /* We're CPU 0. In fact, that's the only CPU possible right now. */
> cpu_id = 0;
> +
> /*
> * We need to know how much memory so we can set up the device
> * descriptor and memory pages for the devices as we parse the command
> @@ -1926,7 +2020,7 @@ int main(int argc, char *argv[])
> */
> tell_kernel(start);
>
> - /* Ensure that we terminate if a child dies. */
> + /* Ensure that we terminate if a device-servicing child dies. */
> signal(SIGCHLD, kill_launcher);
>
> /* If we exit via err(), this kills all the threads, restores tty. */
> diff --git a/arch/x86/include/asm/lguest_hcall.h b/arch/x86/include/asm/lguest_hcall.h
> --- a/arch/x86/include/asm/lguest_hcall.h
> +++ b/arch/x86/include/asm/lguest_hcall.h
> @@ -35,10 +35,10 @@
> * operations? There are two ways: the direct way is to make a "hypercall",
> * to make requests of the Host Itself.
> *
> - * We use the KVM hypercall mechanism. Seventeen hypercalls are
> - * available: the hypercall number is put in the %eax register, and the
> - * arguments (when required) are placed in %ebx, %ecx, %edx and %esi.
> - * If a return value makes sense, it's returned in %eax.
> + * We use the KVM hypercall mechanism, though completely different hypercall
> + * numbers. Seventeen hypercalls are available: the hypercall number is put in
> + * the %eax register, and the arguments (when required) are placed in %ebx,
> + * %ecx, %edx and %esi. If a return value makes sense, it's returned in %eax.
> *
> * Grossly invalid calls result in Sudden Death at the hands of the vengeful
> * Host, rather than returning failure. This reflects Winston Churchill's
> diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
> --- a/arch/x86/lguest/boot.c
> +++ b/arch/x86/lguest/boot.c
> @@ -154,6 +154,7 @@ static void lazy_hcall1(unsigned long ca
> async_hcall(call, arg1, 0, 0, 0);
> }
>
> +/* You can imagine what lazy_hcall2, 3 and 4 look like. :*/
> static void lazy_hcall2(unsigned long call,
> unsigned long arg1,
> unsigned long arg2)
> @@ -189,8 +190,10 @@ static void lazy_hcall4(unsigned long ca
> }
> #endif
>
> -/* When lazy mode is turned off reset the per-cpu lazy mode variable and then
> - * issue the do-nothing hypercall to flush any stored calls. */
> +/*G:036
> + * When lazy mode is turned off reset the per-cpu lazy mode variable and then
> + * issue the do-nothing hypercall to flush any stored calls.
> +:*/
> static void lguest_leave_lazy_mmu_mode(void)
> {
> kvm_hypercall0(LHCALL_FLUSH_ASYNC);
> @@ -250,13 +253,11 @@ extern void lg_irq_enable(void);
> extern void lg_restore_fl(unsigned long flags);
>
> /*M:003
> - * Note that we don't check for outstanding interrupts when we re-enable them
> - * (or when we unmask an interrupt). This seems to work for the moment, since
> - * interrupts are rare and we'll just get the interrupt on the next timer tick,
> - * but now we can run with CONFIG_NO_HZ, we should revisit this. One way would
> - * be to put the "irq_enabled" field in a page by itself, and have the Host
> - * write-protect it when an interrupt comes in when irqs are disabled. There
> - * will then be a page fault as soon as interrupts are re-enabled.
> + * We could be more efficient in our checking of outstanding interrupts, rather
> + * than using a branch. One way would be to put the "irq_enabled" field in a
> + * page by itself, and have the Host write-protect it when an interrupt comes
> + * in when irqs are disabled. There will then be a page fault as soon as
> + * interrupts are re-enabled.
> *
> * A better method is to implement soft interrupt disable generally for x86:
> * instead of disabling interrupts, we set a flag. If an interrupt does come
> @@ -568,7 +569,7 @@ static void lguest_write_cr4(unsigned lo
> * cr3 ---> +---------+
> * | --------->+---------+
> * | | | PADDR1 |
> - * Top-level | | PADDR2 |
> + * Mid-level | | PADDR2 |
> * (PMD) page | | |
> * | | Lower-level |
> * | | (PTE) page |
> @@ -588,23 +589,62 @@ static void lguest_write_cr4(unsigned lo
> * Index into top Index into second Offset within page
> * page directory page pagetable page
> *
> - * The kernel spends a lot of time changing both the top-level page directory
> - * and lower-level pagetable pages. The Guest doesn't know physical addresses,
> - * so while it maintains these page tables exactly like normal, it also needs
> - * to keep the Host informed whenever it makes a change: the Host will create
> - * the real page tables based on the Guests'.
> + * Now, unfortunately, this isn't the whole story: Intel added Physical Address
> + * Extension (PAE) to allow 32 bit systems to use 64GB of memory (ie. 36 bits).
> + * These are held in 64-bit page table entries, so we can now only fit 512
> + * entries in a page, and the neat three-level tree breaks down.
> + *
> + * The result is a four level page table:
> + *
> + * cr3 --> [ 4 Upper ]
> + * [ Level ]
> + * [ Entries ]
> + * [(PUD Page)]---> +---------+
> + * | --------->+---------+
> + * | | | PADDR1 |
> + * Mid-level | | PADDR2 |
> + * (PMD) page | | |
> + * | | Lower-level |
> + * | | (PTE) page |
> + * | | | |
> + * .... ....
> + *
> + *
> + * And the virtual address is decoded as:
> + *
> + * 1 1 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
> + * |<-2->|<--- 9 bits ---->|<---- 9 bits --->|<------ 12 bits ------>|
> + * Index into Index into mid Index into lower Offset within page
> + * top entries directory page pagetable page
> + *
> + * It's too hard to switch between these two formats at runtime, so Linux only
> + * supports one or the other depending on whether CONFIG_X86_PAE is set. Many
> + * distributions turn it on, and not just for people with silly amounts of
> + * memory: the larger PTE entries allow room for the NX bit, which lets the
> + * kernel disable execution of pages and increase security.
> + *
> + * This was a problem for lguest, which couldn't run on these distributions;
> + * then Matias Zabaljauregui figured it all out and implemented it, and only a
> + * handful of puppies were crushed in the process!
> + *
> + * Back to our point: the kernel spends a lot of time changing both the
> + * top-level page directory and lower-level pagetable pages. The Guest doesn't
> + * know physical addresses, so while it maintains these page tables exactly
> + * like normal, it also needs to keep the Host informed whenever it makes a
> + * change: the Host will create the real page tables based on the Guests'.
> */
>
> /*
> - * The Guest calls this to set a second-level entry (pte), ie. to map a page
> - * into a process' address space. We set the entry then tell the Host the
> - * toplevel and address this corresponds to. The Guest uses one pagetable per
> - * process, so we need to tell the Host which one we're changing (mm->pgd).
> + * The Guest calls this after it has set a second-level entry (pte), ie. to map
> + * a page into a process' address space. Wetell the Host the toplevel and
> + * address this corresponds to. The Guest uses one pagetable per process, so
> + * we need to tell the Host which one we're changing (mm->pgd).
> */
> static void lguest_pte_update(struct mm_struct *mm, unsigned long addr,
> pte_t *ptep)
> {
> #ifdef CONFIG_X86_PAE
> + /* PAE needs to hand a 64 bit page table entry, so it uses two args. */
> lazy_hcall4(LHCALL_SET_PTE, __pa(mm->pgd), addr,
> ptep->pte_low, ptep->pte_high);
> #else
> @@ -612,6 +652,7 @@ static void lguest_pte_update(struct mm_
> #endif
> }
>
> +/* This is the "set and update" combo-meal-deal version. */
> static void lguest_set_pte_at(struct mm_struct *mm, unsigned long addr,
> pte_t *ptep, pte_t pteval)
> {
> @@ -672,6 +713,11 @@ static void lguest_set_pte(pte_t *ptep,
> }
>
> #ifdef CONFIG_X86_PAE
> +/*
> + * With 64-bit PTE values, we need to be careful setting them: if we set 32
> + * bits at a time, the hardware could see a weird half-set entry. These
> + * versions ensure we update all 64 bits at once.
> + */
> static void lguest_set_pte_atomic(pte_t *ptep, pte_t pte)
> {
> native_set_pte_atomic(ptep, pte);
> @@ -679,13 +725,14 @@ static void lguest_set_pte_atomic(pte_t
> lazy_hcall1(LHCALL_FLUSH_TLB, 1);
> }
>
> -void lguest_pte_clear(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
> +static void lguest_pte_clear(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep)
> {
> native_pte_clear(mm, addr, ptep);
> lguest_pte_update(mm, addr, ptep);
> }
>
> -void lguest_pmd_clear(pmd_t *pmdp)
> +static void lguest_pmd_clear(pmd_t *pmdp)
> {
> lguest_set_pmd(pmdp, __pmd(0));
> }
> @@ -784,6 +831,14 @@ static void __init lguest_init_IRQ(void)
> irq_ctx_init(smp_processor_id());
> }
>
> +/*
> + * With CONFIG_SPARSE_IRQ, interrupt descriptors are allocated as-needed, so
> + * rather than set them in lguest_init_IRQ we are called here every time an
> + * lguest device needs an interrupt.
> + *
> + * FIXME: irq_to_desc_alloc_node() can fail due to lack of memory, we should
> + * pass that up!
> + */
> void lguest_setup_irq(unsigned int irq)
> {
> irq_to_desc_alloc_node(irq, 0);
> @@ -1298,7 +1353,7 @@ __init void lguest_init(void)
> */
> switch_to_new_gdt(0);
>
> - /* As described in head_32.S, we map the first 128M of memory. */
> + /* We actually boot with all memory mapped, but let's say 128MB. */
> max_pfn_mapped = (128*1024*1024) >> PAGE_SHIFT;
>
> /*
> diff --git a/arch/x86/lguest/i386_head.S b/arch/x86/lguest/i386_head.S
> --- a/arch/x86/lguest/i386_head.S
> +++ b/arch/x86/lguest/i386_head.S
> @@ -102,6 +102,7 @@ send_interrupts:
> * create one manually here.
> */
> .byte 0x0f,0x01,0xc1 /* KVM_HYPERCALL */
> + /* Put eax back the way we found it. */
> popl %eax
> ret
>
> @@ -125,6 +126,7 @@ ENTRY(lg_restore_fl)
> jnz send_interrupts
> /* Again, the normal path has used no extra registers. Clever, huh? */
> ret
> +/*:*/
>
> /* These demark the EIP range where host should never deliver interrupts. */
> .global lguest_noirq_start
> diff --git a/drivers/lguest/core.c b/drivers/lguest/core.c
> --- a/drivers/lguest/core.c
> +++ b/drivers/lguest/core.c
> @@ -217,10 +217,15 @@ int run_guest(struct lg_cpu *cpu, unsign
>
> /*
> * It's possible the Guest did a NOTIFY hypercall to the
> - * Launcher, in which case we return from the read() now.
> + * Launcher.
> */
> if (cpu->pending_notify) {
> + /*
> + * Does it just needs to write to a registered
> + * eventfd (ie. the appropriate virtqueue thread)?
> + */
> if (!send_notify_to_eventfd(cpu)) {
> + /* OK, we tell the main Laucher. */
> if (put_user(cpu->pending_notify, user))
> return -EFAULT;
> return sizeof(cpu->pending_notify);
> diff --git a/drivers/lguest/hypercalls.c b/drivers/lguest/hypercalls.c
> --- a/drivers/lguest/hypercalls.c
> +++ b/drivers/lguest/hypercalls.c
> @@ -59,7 +59,7 @@ static void do_hcall(struct lg_cpu *cpu,
> case LHCALL_SHUTDOWN: {
> char msg[128];
> /*
> - * Shutdown is such a trivial hypercall that we do it in four
> + * Shutdown is such a trivial hypercall that we do it in five
> * lines right here.
> *
> * If the lgread fails, it will call kill_guest() itself; the
> @@ -245,6 +245,10 @@ static void initialize(struct lg_cpu *cp
> * device), the Guest will still see the old page. In practice, this never
> * happens: why would the Guest read a page which it has never written to? But
> * a similar scenario might one day bite us, so it's worth mentioning.
> + *
> + * Note that if we used a shared anonymous mapping in the Launcher instead of
> + * mapping /dev/zero private, we wouldn't worry about cop-on-write. And we
> + * need that to switch the Launcher to processes (away from threads) anyway.
> :*/
>
> /*H:100
> diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
> --- a/drivers/lguest/lguest_device.c
> +++ b/drivers/lguest/lguest_device.c
> @@ -236,7 +236,7 @@ static void lg_notify(struct virtqueue *
> extern void lguest_setup_irq(unsigned int irq);
>
> /*
> - * This routine finds the first virtqueue described in the configuration of
> + * This routine finds the Nth virtqueue described in the configuration of
> * this device and sets it up.
> *
> * This is kind of an ugly duckling. It'd be nicer to have a standard
> @@ -244,9 +244,6 @@ extern void lguest_setup_irq(unsigned in
> * everyone wants to do it differently. The KVM coders want the Guest to
> * allocate its own pages and tell the Host where they are, but for lguest it's
> * simpler for the Host to simply tell us where the pages are.
> - *
> - * So we provide drivers with a "find the Nth virtqueue and set it up"
> - * function.
> */
> static struct virtqueue *lg_find_vq(struct virtio_device *vdev,
> unsigned index,
> @@ -422,7 +419,11 @@ static void add_lguest_device(struct lgu
>
> /* This devices' parent is the lguest/ dir. */
> ldev->vdev.dev.parent = lguest_root;
> - /* We have a unique device index thanks to the dev_index counter. */
> + /*
> + * The device type comes straight from the descriptor. There's also a
> + * device vendor field in the virtio_device struct, which we leave as
> + * 0.
> + */
> ldev->vdev.id.device = d->type;
> /*
> * We have a simple set of routines for querying the device's
> diff --git a/drivers/lguest/lguest_user.c b/drivers/lguest/lguest_user.c
> --- a/drivers/lguest/lguest_user.c
> +++ b/drivers/lguest/lguest_user.c
> @@ -32,10 +32,36 @@ bool send_notify_to_eventfd(struct lg_cp
> return cpu->pending_notify == 0;
> }
>
> +/*L:055
> + * One of the more tricksy tricks in the Linux Kernel is a technique called
> + * Read Copy Update. Since one point of lguest is to teach lguest journeyers
> + * about kernel coding, I use it here. (In case you're curious, other purposes
> + * include learning about virtualization and instilling a deep appreciation for
> + * simplicity and puppies).
> + *
> + * We keep a simple array which maps LHCALL_NOTIFY values to eventfds, but we
> + * we add new eventfds without ever blocking readers from accessing the array.
Suggest s/we we/we/ ;-)
> + * The current Launcher only does this during boot, so that never happens. But
> + * Read Copy Update is cool, and adding a lock risks damaging even more puppies
> + * than this code does.
> + *
> + * We allocate a brand new one-larger array, copy the old one and add our new
> + * element. Then we make the lg eventfd pointer point to the new array.
Could you please also mention the role of rcu_assign_pointer()?
> + * That's the easy part: now we need to free the old one, but we need to make
> + * sure no slow CPU somewhere is still looking at it. That's what
> + * synchronize_rcu does for us: waits until every CPU has indicated that it has
> + * moved on to know it's no longer using the old one.
This is protected on by lguest_lock, correct?
> + *
> + * If that's unclear, see http://en.wikipedia.org/wiki/Read-copy-update.
> + */
> static int add_eventfd(struct lguest *lg, unsigned long addr, int fd)
> {
> struct lg_eventfd_map *new, *old = lg->eventfds;
>
> + /*
> + * We don't allow notifications on address 0 anyway (pending_notify of
> + * 0 means "nothing pending).
s/"nothing pending/"nothing pending"/
> + */
> if (!addr)
> return -EINVAL;
>
> @@ -75,6 +101,14 @@ static int add_eventfd(struct lguest *lg
> return 0;
> }
>
> +/*L:052
> + * Receiving notifications from the Guest is usually done by attaching a
> + * particular LHCALL_NOTIFY value to an event filedescriptor. The eventfd will
> + * become readable when the Guest does an LHCALL_NOTIFY with that value.
> + *
> + * This is really convenient for processing each virtqueue in a separate
> + * thread.
> + */
> static int attach_eventfd(struct lguest *lg, const unsigned long __user *input)
> {
> unsigned long addr, fd;
> @@ -86,6 +120,11 @@ static int attach_eventfd(struct lguest
> if (get_user(fd, input) != 0)
> return -EFAULT;
>
> + /*
> + * Just make sure two callers don't add eventfds at once. We really
> + * only need to lock against callers adding to the same Guest, so using
> + * the Big Lguest Lock is overkill. But this is setup, not a fast path.
> + */
> mutex_lock(&lguest_lock);
> err = add_eventfd(lg, addr, fd);
> mutex_unlock(&lguest_lock);
> @@ -106,6 +145,10 @@ static int user_send_irq(struct lg_cpu *
> if (irq >= LGUEST_IRQS)
> return -EINVAL;
>
> + /*
> + * Next time the Guest runs, the core code will see if it can deliver
> + * this interrupt.
> + */
> set_interrupt(cpu, irq);
> return 0;
> }
> @@ -307,10 +350,10 @@ unlock:
> * The first operation the Launcher does must be a write. All writes
> * start with an unsigned long number: for the first write this must be
> * LHREQ_INITIALIZE to set up the Guest. After that the Launcher can use
> - * writes of other values to send interrupts.
> + * writes of other values to send interrupts or set up receipt of notifications.
> *
> * Note that we overload the "offset" in the /dev/lguest file to indicate what
> - * CPU number we're dealing with. Currently this is always 0, since we only
> + * CPU number we're dealing with. Currently this is always 0 since we only
> * support uniprocessor Guests, but you can see the beginnings of SMP support
> * here.
> */
> diff --git a/drivers/lguest/page_tables.c b/drivers/lguest/page_tables.c
> --- a/drivers/lguest/page_tables.c
> +++ b/drivers/lguest/page_tables.c
> @@ -29,10 +29,10 @@
> /*H:300
> * The Page Table Code
> *
> - * We use two-level page tables for the Guest. If you're not entirely
> - * comfortable with virtual addresses, physical addresses and page tables then
> - * I recommend you review arch/x86/lguest/boot.c's "Page Table Handling" (with
> - * diagrams!).
> + * We use two-level page tables for the Guest, or three-level with PAE. If
> + * you're not entirely comfortable with virtual addresses, physical addresses
> + * and page tables then I recommend you review arch/x86/lguest/boot.c's "Page
> + * Table Handling" (with diagrams!).
> *
> * The Guest keeps page tables, but we maintain the actual ones here: these are
> * called "shadow" page tables. Which is a very Guest-centric name: these are
> @@ -52,9 +52,8 @@
> :*/
>
> /*
> - * 1024 entries in a page table page maps 1024 pages: 4MB. The Switcher is
> - * conveniently placed at the top 4MB, so it uses a separate, complete PTE
> - * page.
> + * The Switcher uses the complete top PTE page. That's 1024 PTE entries (4MB)
> + * or 512 PTE entries with PAE (2MB).
> */
> #define SWITCHER_PGD_INDEX (PTRS_PER_PGD - 1)
>
> @@ -81,7 +80,8 @@ static DEFINE_PER_CPU(pte_t *, switcher_
>
> /*H:320
> * The page table code is curly enough to need helper functions to keep it
> - * clear and clean.
> + * clear and clean. The kernel itself provides many of them; one advantage
> + * of insisting that the Guest and Host use the same CONFIG_PAE setting.
> *
> * There are two functions which return pointers to the shadow (aka "real")
> * page tables.
> @@ -155,7 +155,7 @@ static pte_t *spte_addr(struct lg_cpu *c
> }
>
> /*
> - * These two functions just like the above two, except they access the Guest
> + * These functions are just like the above two, except they access the Guest
> * page tables. Hence they return a Guest address.
> */
> static unsigned long gpgd_addr(struct lg_cpu *cpu, unsigned long vaddr)
> @@ -165,6 +165,7 @@ static unsigned long gpgd_addr(struct lg
> }
>
> #ifdef CONFIG_X86_PAE
> +/* Follow the PGD to the PMD. */
> static unsigned long gpmd_addr(pgd_t gpgd, unsigned long vaddr)
> {
> unsigned long gpage = pgd_pfn(gpgd) << PAGE_SHIFT;
> @@ -172,6 +173,7 @@ static unsigned long gpmd_addr(pgd_t gpg
> return gpage + pmd_index(vaddr) * sizeof(pmd_t);
> }
>
> +/* Follow the PMD to the PTE. */
> static unsigned long gpte_addr(struct lg_cpu *cpu,
> pmd_t gpmd, unsigned long vaddr)
> {
> @@ -181,6 +183,7 @@ static unsigned long gpte_addr(struct lg
> return gpage + pte_index(vaddr) * sizeof(pte_t);
> }
> #else
> +/* Follow the PGD to the PTE (no mid-level for !PAE). */
> static unsigned long gpte_addr(struct lg_cpu *cpu,
> pgd_t gpgd, unsigned long vaddr)
> {
> @@ -314,6 +317,7 @@ bool demand_page(struct lg_cpu *cpu, uns
> pte_t gpte;
> pte_t *spte;
>
> + /* Mid level for PAE. */
> #ifdef CONFIG_X86_PAE
> pmd_t *spmd;
> pmd_t gpmd;
> @@ -391,6 +395,8 @@ bool demand_page(struct lg_cpu *cpu, uns
> */
> gpte_ptr = gpte_addr(cpu, gpgd, vaddr);
> #endif
> +
> + /* Read the actual PTE value. */
> gpte = lgread(cpu, gpte_ptr, pte_t);
>
> /* If this page isn't in the Guest page tables, we can't page it in. */
> @@ -507,6 +513,7 @@ void pin_page(struct lg_cpu *cpu, unsign
> if (!page_writable(cpu, vaddr) && !demand_page(cpu, vaddr, 2))
> kill_guest(cpu, "bad stack page %#lx", vaddr);
> }
> +/*:*/
>
> #ifdef CONFIG_X86_PAE
> static void release_pmd(pmd_t *spmd)
> @@ -543,7 +550,11 @@ static void release_pgd(pgd_t *spgd)
> }
>
> #else /* !CONFIG_X86_PAE */
> -/*H:450 If we chase down the release_pgd() code, it looks like this: */
> +/*H:450
> + * If we chase down the release_pgd() code, the non-PAE version looks like
> + * this. The PAE version is almost identical, but instead of calling
> + * release_pte it calls release_pmd(), which looks much like this.
> + */
> static void release_pgd(pgd_t *spgd)
> {
> /* If the entry's not present, there's nothing to release. */
> @@ -898,17 +909,21 @@ void guest_set_pgd(struct lguest *lg, un
> /* ... throw it away. */
> release_pgd(lg->pgdirs[pgdir].pgdir + idx);
> }
> +
> #ifdef CONFIG_X86_PAE
> +/* For setting a mid-level, we just throw everything away. It's easy. */
> void guest_set_pmd(struct lguest *lg, unsigned long pmdp, u32 idx)
> {
> guest_pagetable_clear_all(&lg->cpus[0]);
> }
> #endif
>
> -/*
> - * Once we know how much memory we have we can construct simple identity (which
> +/*H:505
> + * To get through boot, we construct simple identity page mappings (which
> * set virtual == physical) and linear mappings which will get the Guest far
> - * enough into the boot to create its own.
> + * enough into the boot to create its own. The linear mapping means we
> + * simplify the Guest boot, but it makes assumptions about their PAGE_OFFSET,
> + * as you'll see.
> *
> * We lay them out of the way, just below the initrd (which is why we need to
> * know its size here).
> @@ -944,6 +959,10 @@ static unsigned long setup_pagetables(st
> linear = (void *)pgdir - linear_pages * PAGE_SIZE;
>
> #ifdef CONFIG_X86_PAE
> + /*
> + * And the single mid page goes below that. We only use one, but
> + * that's enough to map 1G, which definitely gets us through boot.
> + */
> pmds = (void *)linear - PAGE_SIZE;
> #endif
> /*
> @@ -957,13 +976,14 @@ static unsigned long setup_pagetables(st
> return -EFAULT;
> }
>
> +#ifdef CONFIG_X86_PAE
> /*
> - * The top level points to the linear page table pages above.
> - * We setup the identity and linear mappings here.
> + * Make the Guest PMD entries point to the corresponding place in the
> + * linear mapping (up to one page worth of PMD).
> */
> -#ifdef CONFIG_X86_PAE
> for (i = j = 0; i < mapped_pages && j < PTRS_PER_PMD;
> i += PTRS_PER_PTE, j++) {
> + /* FIXME: native_set_pmd is overkill here. */
> native_set_pmd(&pmd, __pmd(((unsigned long)(linear + i)
> - mem_base) | _PAGE_PRESENT | _PAGE_RW | _PAGE_USER));
>
> @@ -971,18 +991,36 @@ static unsigned long setup_pagetables(st
> return -EFAULT;
> }
>
> + /* One PGD entry, pointing to that PMD page. */
> set_pgd(&pgd, __pgd(((u32)pmds - mem_base) | _PAGE_PRESENT));
> + /* Copy it in as the first PGD entry (ie. addresses 0-1G). */
> if (copy_to_user(&pgdir[0], &pgd, sizeof(pgd)) != 0)
> return -EFAULT;
> + /*
> + * And the third PGD entry (ie. addresses 3G-4G).
> + *
> + * FIXME: This assumes that PAGE_OFFSET for the Guest is 0xC0000000.
> + */
> if (copy_to_user(&pgdir[3], &pgd, sizeof(pgd)) != 0)
> return -EFAULT;
> #else
> + /*
> + * The top level points to the linear page table pages above.
> + * We setup the identity and linear mappings here.
> + */
> phys_linear = (unsigned long)linear - mem_base;
> for (i = 0; i < mapped_pages; i += PTRS_PER_PTE) {
> pgd_t pgd;
> + /*
> + * Create a PGD entry which points to the right part of the
> + * linear PTE pages.
> + */
> pgd = __pgd((phys_linear + i * sizeof(pte_t)) |
> (_PAGE_PRESENT | _PAGE_RW | _PAGE_USER));
>
> + /*
> + * Copy it into the PGD page at 0 and PAGE_OFFSET.
> + */
> if (copy_to_user(&pgdir[i / PTRS_PER_PTE], &pgd, sizeof(pgd))
> || copy_to_user(&pgdir[pgd_index(PAGE_OFFSET)
> + i / PTRS_PER_PTE],
> @@ -992,8 +1030,8 @@ static unsigned long setup_pagetables(st
> #endif
>
> /*
> - * We return the top level (guest-physical) address: remember where
> - * this is.
> + * We return the top level (guest-physical) address: we remember where
> + * this is to write it into lguest_data when the Guest initializes.
> */
> return (unsigned long)pgdir - mem_base;
> }
> @@ -1031,7 +1069,9 @@ int init_guest_pagetable(struct lguest *
> lg->pgdirs[0].pgdir = (pgd_t *)get_zeroed_page(GFP_KERNEL);
> if (!lg->pgdirs[0].pgdir)
> return -ENOMEM;
> +
> #ifdef CONFIG_X86_PAE
> + /* For PAE, we also create the initial mid-level. */
> pgd = lg->pgdirs[0].pgdir;
> pmd_table = (pmd_t *) get_zeroed_page(GFP_KERNEL);
> if (!pmd_table)
> @@ -1040,11 +1080,13 @@ int init_guest_pagetable(struct lguest *
> set_pgd(pgd + SWITCHER_PGD_INDEX,
> __pgd(__pa(pmd_table) | _PAGE_PRESENT));
> #endif
> +
> + /* This is the current page table. */
> lg->cpus[0].cpu_pgd = 0;
> return 0;
> }
>
> -/* When the Guest calls LHCALL_LGUEST_INIT we do more setup. */
> +/*H:508 When the Guest calls LHCALL_LGUEST_INIT we do more setup. */
> void page_table_guest_data_init(struct lg_cpu *cpu)
> {
> /* We get the kernel address: above this is all kernel memory. */
> @@ -1105,12 +1147,16 @@ void map_switcher_in_guest(struct lg_cpu
> pmd_t switcher_pmd;
> pmd_t *pmd_table;
>
> + /* FIXME: native_set_pmd is overkill here. */
> native_set_pmd(&switcher_pmd, pfn_pmd(__pa(switcher_pte_page) >>
> PAGE_SHIFT, PAGE_KERNEL_EXEC));
>
> + /* Figure out where the pmd page is, by reading the PGD, and converting
> + * it to a virtual address. */
> pmd_table = __va(pgd_pfn(cpu->lg->
> pgdirs[cpu->cpu_pgd].pgdir[SWITCHER_PGD_INDEX])
> << PAGE_SHIFT);
> + /* Now write it into the shadow page table. */
> native_set_pmd(&pmd_table[SWITCHER_PMD_INDEX], switcher_pmd);
> #else
> pgd_t switcher_pgd;
> diff --git a/drivers/lguest/x86/core.c b/drivers/lguest/x86/core.c
> --- a/drivers/lguest/x86/core.c
> +++ b/drivers/lguest/x86/core.c
> @@ -187,7 +187,7 @@ static void run_guest_once(struct lg_cpu
> * also simplify copy_in_guest_info(). Note that we'd still need to restore
> * things when we exit to Launcher userspace, but that's fairly easy.
> *
> - * We could also try using this hooks for PGE, but that might be too expensive.
> + * We could also try using these hooks for PGE, but that might be too expensive.
> *
> * The hooks were designed for KVM, but we can also put them to good use.
> :*/
> diff --git a/drivers/lguest/x86/switcher_32.S b/drivers/lguest/x86/switcher_32.S
> --- a/drivers/lguest/x86/switcher_32.S
> +++ b/drivers/lguest/x86/switcher_32.S
> @@ -1,7 +1,7 @@
> /*P:900
> - * This is the Switcher: code which sits at 0xFFC00000 astride both the
> - * Host and Guest to do the low-level Guest<->Host switch. It is as simple as
> - * it can be made, but it's naturally very specific to x86.
> + * This is the Switcher: code which sits at 0xFFC00000 (or 0xFFE00000) astride
> + * both the Host and Guest to do the low-level Guest<->Host switch. It is as
> + * simple as it can be made, but it's naturally very specific to x86.
> *
> * You have now completed Preparation. If this has whet your appetite; if you
> * are feeling invigorated and refreshed then the next, more challenging stage
>
More information about the Lguest
mailing list