* [PATCH] abstract out bits of ldt.c
@ 2005-08-07 23:20 Martin J. Bligh
2005-08-07 23:44 ` Chris Wright
2005-08-08 8:41 ` Dave Hansen
0 siblings, 2 replies; 15+ messages in thread
From: Martin J. Bligh @ 2005-08-07 23:20 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
Starting on the work to merge xen cleanly as a subarch.
Introduce make_pages_readonly and make_pages_writable where appropriate
for Xen, defined as a no-op on other subarches. Same for
add_context_to_unpinned and del_context_from_unpinned.
Abstract out install_ldt_entry().
This will do have no effect whatsover on platforms other than xen.
Compile-tested with every config I can find. Boot tested on i386.
diff -aurpN -X /home/fletch/.diff.exclude virgin/arch/i386/kernel/ldt.c xen-ldt.c/arch/i386/kernel/ldt.c
--- virgin/arch/i386/kernel/ldt.c 2004-10-24 19:27:13.000000000 -0700
+++ xen-ldt.c/arch/i386/kernel/ldt.c 2005-08-07 10:45:49.000000000 -0700
@@ -18,6 +18,7 @@
#include <asm/system.h>
#include <asm/ldt.h>
#include <asm/desc.h>
+#include <mach_ldt.h>
#ifdef CONFIG_SMP /* avoids "defined but not used" warnig */
static void flush_ldt(void *null)
@@ -58,16 +59,22 @@ static int alloc_ldt(mm_context_t *pc, i
#ifdef CONFIG_SMP
cpumask_t mask;
preempt_disable();
+ make_pages_readonly(pc->ldt, (pc->size * LDT_ENTRY_SIZE) /
+ PAGE_SIZE);
load_LDT(pc);
mask = cpumask_of_cpu(smp_processor_id());
if (!cpus_equal(current->mm->cpu_vm_mask, mask))
smp_call_function(flush_ldt, NULL, 1, 1);
preempt_enable();
#else
+ make_pages_readonly(pc->ldt, (pc->size * LDT_ENTRY_SIZE) /
+ PAGE_SIZE);
load_LDT(pc);
#endif
}
if (oldsize) {
+ make_pages_writable(oldldt, (oldsize * LDT_ENTRY_SIZE) /
+ PAGE_SIZE);
if (oldsize*LDT_ENTRY_SIZE > PAGE_SIZE)
vfree(oldldt);
else
@@ -82,6 +89,8 @@ static inline int copy_ldt(mm_context_t
if (err < 0)
return err;
memcpy(new->ldt, old->ldt, old->size*LDT_ENTRY_SIZE);
+ make_pages_readonly(new->ldt, (new->size * LDT_ENTRY_SIZE) /
+ PAGE_SIZE);
return 0;
}
@@ -94,14 +103,16 @@ int init_new_context(struct task_struct
struct mm_struct * old_mm;
int retval = 0;
+ memset(&mm->context, 0, sizeof(mm->context));
init_MUTEX(&mm->context.sem);
- mm->context.size = 0;
old_mm = current->mm;
if (old_mm && old_mm->context.size > 0) {
down(&old_mm->context.sem);
retval = copy_ldt(&mm->context, &old_mm->context);
up(&old_mm->context.sem);
}
+ if (retval == 0)
+ add_context_to_unpinned(mm);
return retval;
}
@@ -113,12 +124,16 @@ void destroy_context(struct mm_struct *m
if (mm->context.size) {
if (mm == current->active_mm)
clear_LDT();
+ make_pages_writable(mm->context.ldt,
+ (mm->context.size * LDT_ENTRY_SIZE) /
+ PAGE_SIZE);
if (mm->context.size*LDT_ENTRY_SIZE > PAGE_SIZE)
vfree(mm->context.ldt);
else
kfree(mm->context.ldt);
mm->context.size = 0;
}
+ del_context_from_unpinned(mm);
}
static int read_ldt(void __user * ptr, unsigned long bytecount)
@@ -223,9 +238,7 @@ static int write_ldt(void __user * ptr,
/* Install the new entry ... */
install:
- *lp = entry_1;
- *(lp+1) = entry_2;
- error = 0;
+ error = install_ldt_entry(lp, entry_1, entry_2);
out_unlock:
up(&mm->context.sem);
diff -aurpN -X /home/fletch/.diff.exclude virgin/include/asm-i386/mach-default/mach_ldt.h xen-ldt.c/include/asm-i386/mach-default/mach_ldt.h
--- virgin/include/asm-i386/mach-default/mach_ldt.h 1969-12-31 16:00:00.000000000 -0800
+++ xen-ldt.c/include/asm-i386/mach-default/mach_ldt.h 2005-08-07 10:43:58.000000000 -0700
@@ -0,0 +1,27 @@
+#ifndef __ASM_MACH_LDT_H
+#define __ASM_MACH_LDT_H
+
+static inline void make_pages_readonly(void *va, unsigned int nr)
+{
+}
+
+static inline void make_pages_writable(void *va, unsigned int nr)
+{
+}
+
+static inline void add_context_to_unpinned(struct mm_struct *mm)
+{
+}
+
+static inline void del_context_from_unpinned(struct mm_struct *mm)
+{
+}
+
+static inline int install_ldt_entry (__u32 *lp, __u32 entry_1, __u32 entry_2)
+{
+ *lp = entry_1;
+ *(lp+1) = entry_2;
+ return 0;
+}
+
+#endif /* __ASM_MACH_LDT_H */
diff -aurpN -X /home/fletch/.diff.exclude virgin/include/asm-i386/mach-xen/mach_ldt.h xen-ldt.c/include/asm-i386/mach-xen/mach_ldt.h
--- virgin/include/asm-i386/mach-xen/mach_ldt.h 1969-12-31 16:00:00.000000000 -0800
+++ xen-ldt.c/include/asm-i386/mach-xen/mach_ldt.h 2005-08-07 10:44:45.000000000 -0700
@@ -0,0 +1,27 @@
+#ifndef __ASM_MACH_LDT_H
+#define __ASM_MACH_LDT_H
+
+static inline void add_context_to_unpinned(struct mm_struct *mm)
+{
+ spin_lock(&mm_unpinned_lock);
+ list_add(&mm->context.unpinned, &mm_unpinned);
+ spin_unlock(&mm_unpinned_lock);
+}
+
+static inline void del_context_from_unpinned(struct mm_struct *mm)
+{
+ if (!mm->context.pinned) {
+ spin_lock(&mm_unpinned_lock);
+ list_del(&mm->context.unpinned);
+ spin_unlock(&mm_unpinned_lock);
+ }
+}
+
+static inline int install_ldt_entry (__u32 *lp, __u32 entry_1, __u32 entry_2)
+{
+ unsigned long mach_lp = arbitrary_virt_to_machine(lp);
+
+ return HYPERVISOR_update_descriptor(mach_lp, entry_1, entry_2);
+}
+
+#endif /* __ASM_MACH_LDT_H */
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-07 23:20 [PATCH] abstract out bits of ldt.c Martin J. Bligh
@ 2005-08-07 23:44 ` Chris Wright
2005-08-07 23:57 ` Martin J. Bligh
2005-08-08 8:41 ` Dave Hansen
1 sibling, 1 reply; 15+ messages in thread
From: Chris Wright @ 2005-08-07 23:44 UTC (permalink / raw)
To: Martin J. Bligh; +Cc: Andrew Morton, linux-kernel
* Martin J. Bligh (mbligh@mbligh.org) wrote:
> Starting on the work to merge xen cleanly as a subarch.
> Introduce make_pages_readonly and make_pages_writable where appropriate
> for Xen, defined as a no-op on other subarches. Same for
Maybe this is a bad name, since make_pages_readonly/writable has
intutitive meaning, and then is non-inutitively a no-op (for default).
thanks,
-chris
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-07 23:44 ` Chris Wright
@ 2005-08-07 23:57 ` Martin J. Bligh
2005-08-07 23:59 ` Chris Wright
2005-08-08 0:41 ` Andrew Morton
0 siblings, 2 replies; 15+ messages in thread
From: Martin J. Bligh @ 2005-08-07 23:57 UTC (permalink / raw)
To: Chris Wright; +Cc: Andrew Morton, linux-kernel
--Chris Wright <chrisw@osdl.org> wrote (on Sunday, August 07, 2005 16:44:11 -0700):
> * Martin J. Bligh (mbligh@mbligh.org) wrote:
>> Starting on the work to merge xen cleanly as a subarch.
>> Introduce make_pages_readonly and make_pages_writable where appropriate
>> for Xen, defined as a no-op on other subarches. Same for
>
> Maybe this is a bad name, since make_pages_readonly/writable has
> intutitive meaning, and then is non-inutitively a no-op (for default).
You're welcome to suggest something else if you want, though it would
have been easier if you'd done it the first time you saw this patch,
not now. Going through this stuff multiple times is going to get very
boring very fast.
xen_make_pages_readonly / xen_make_pages_writable ?
M.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-07 23:57 ` Martin J. Bligh
@ 2005-08-07 23:59 ` Chris Wright
2005-08-08 0:41 ` Andrew Morton
1 sibling, 0 replies; 15+ messages in thread
From: Chris Wright @ 2005-08-07 23:59 UTC (permalink / raw)
To: Martin J. Bligh; +Cc: Chris Wright, Andrew Morton, linux-kernel
* Martin J. Bligh (mbligh@mbligh.org) wrote:
> You're welcome to suggest something else if you want, though it would
> have been easier if you'd done it the first time you saw this patch,
> not now. Going through this stuff multiple times is going to get very
> boring very fast.
Sorry, that's my fault, didn't strike me until looking at it this time.
thanks,
-chris
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-07 23:57 ` Martin J. Bligh
2005-08-07 23:59 ` Chris Wright
@ 2005-08-08 0:41 ` Andrew Morton
2005-08-08 0:46 ` Chris Wright
` (2 more replies)
1 sibling, 3 replies; 15+ messages in thread
From: Andrew Morton @ 2005-08-08 0:41 UTC (permalink / raw)
To: Martin J. Bligh; +Cc: chrisw, linux-kernel, Zachary Amsden
"Martin J. Bligh" <mbligh@mbligh.org> wrote:
>
>
>
> --Chris Wright <chrisw@osdl.org> wrote (on Sunday, August 07, 2005 16:44:11 -0700):
>
> > * Martin J. Bligh (mbligh@mbligh.org) wrote:
> >> Starting on the work to merge xen cleanly as a subarch.
> >> Introduce make_pages_readonly and make_pages_writable where appropriate
> >> for Xen, defined as a no-op on other subarches. Same for
> >
> > Maybe this is a bad name, since make_pages_readonly/writable has
> > intutitive meaning, and then is non-inutitively a no-op (for default).
>
> You're welcome to suggest something else if you want, though it would
> have been easier if you'd done it the first time you saw this patch,
> not now. Going through this stuff multiple times is going to get very
> boring very fast.
>
> xen_make_pages_readonly / xen_make_pages_writable ?
>
Well we don't want to assume "xen" at this stage. We're faced with a
choice at present: to make the linux->hypervisor interface be some
xen-specific and xen-controlled thing, or to make it a more formal and
controlled kernel interface which talks to a generic hypervisor rather than
assuming it's Xen down there.
As long as it doesn't hamper performance or general code sanity, I think it
would be better to make this a well-defined and controlled Linux interface.
Some of the code to do that is starting to accumulate in -mm. Everyone
needs to sit down, take a look at the patches and the proposal and work out
if this is the way to proceed.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-08 0:41 ` Andrew Morton
@ 2005-08-08 0:46 ` Chris Wright
2005-08-08 1:04 ` Zachary Amsden
2005-08-08 0:59 ` Martin J. Bligh
[not found] ` <20050808113014.GA15165@elte.hu>
2 siblings, 1 reply; 15+ messages in thread
From: Chris Wright @ 2005-08-08 0:46 UTC (permalink / raw)
To: Andrew Morton; +Cc: Martin J. Bligh, chrisw, linux-kernel, Zachary Amsden
* Andrew Morton (akpm@osdl.org) wrote:
> "Martin J. Bligh" <mbligh@mbligh.org> wrote:
> > xen_make_pages_readonly / xen_make_pages_writable ?
>
> Well we don't want to assume "xen" at this stage. We're faced with a
> choice at present: to make the linux->hypervisor interface be some
> xen-specific and xen-controlled thing, or to make it a more formal and
> controlled kernel interface which talks to a generic hypervisor rather than
> assuming it's Xen down there.
No, definietly not. Xen is not appropriate global namespace. Also,
it's not about pages at this point, it's about ldt handling.
> As long as it doesn't hamper performance or general code sanity, I think it
> would be better to make this a well-defined and controlled Linux interface.
> Some of the code to do that is starting to accumulate in -mm. Everyone
> needs to sit down, take a look at the patches and the proposal and work out
> if this is the way to proceed.
We're doing that, but it's splintered and coming in from different angles.
It'd be better to get the story straight then submit patches, IMO.
thanks,
-chris
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-08 0:41 ` Andrew Morton
2005-08-08 0:46 ` Chris Wright
@ 2005-08-08 0:59 ` Martin J. Bligh
[not found] ` <20050808113014.GA15165@elte.hu>
2 siblings, 0 replies; 15+ messages in thread
From: Martin J. Bligh @ 2005-08-08 0:59 UTC (permalink / raw)
To: Andrew Morton; +Cc: chrisw, linux-kernel, Zachary Amsden
--Andrew Morton <akpm@osdl.org> wrote (on Sunday, August 07, 2005 17:41:29 -0700):
> "Martin J. Bligh" <mbligh@mbligh.org> wrote:
>>
>>
>>
>> --Chris Wright <chrisw@osdl.org> wrote (on Sunday, August 07, 2005 16:44:11 -0700):
>>
>> > * Martin J. Bligh (mbligh@mbligh.org) wrote:
>> >> Starting on the work to merge xen cleanly as a subarch.
>> >> Introduce make_pages_readonly and make_pages_writable where appropriate
>> >> for Xen, defined as a no-op on other subarches. Same for
>> >
>> > Maybe this is a bad name, since make_pages_readonly/writable has
>> > intutitive meaning, and then is non-inutitively a no-op (for default).
>>
>> You're welcome to suggest something else if you want, though it would
>> have been easier if you'd done it the first time you saw this patch,
>> not now. Going through this stuff multiple times is going to get very
>> boring very fast.
>>
>> xen_make_pages_readonly / xen_make_pages_writable ?
>>
>
> Well we don't want to assume "xen" at this stage. We're faced with a
> choice at present: to make the linux->hypervisor interface be some
> xen-specific and xen-controlled thing, or to make it a more formal and
> controlled kernel interface which talks to a generic hypervisor rather than
> assuming it's Xen down there.
Yes, more generic than xen would be better. I think mach-xen is probably
OK for now, but I agree we should avoid wedging xen_* in generic code
callouts. What I'm trying to do right now is rip the whole duplicated
files out of the xen patches.
> As long as it doesn't hamper performance or general code sanity, I think it
> would be better to make this a well-defined and controlled Linux interface.
> Some of the code to do that is starting to accumulate in -mm. Everyone
> needs to sit down, take a look at the patches and the proposal and work out
> if this is the way to proceed.
We're faced with a difficult choice - the xen patches are hard to read
without refactoring them. On the other hand it's going to be very painful
to keep them in sync out of tree whilst doing the refactoring. I'd prefer
to merge up some bits as we go, though maybe I should keep those more
neutral than this. Sigh ... this is going to be extremely painful whatever
we do.
M.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-08 0:46 ` Chris Wright
@ 2005-08-08 1:04 ` Zachary Amsden
2005-08-08 1:08 ` Chris Wright
2005-08-08 1:21 ` Martin J. Bligh
0 siblings, 2 replies; 15+ messages in thread
From: Zachary Amsden @ 2005-08-08 1:04 UTC (permalink / raw)
To: Chris Wright
Cc: Andrew Morton, Martin J. Bligh, linux-kernel, Pratap Subrahmanyam
Chris Wright wrote:
>* Andrew Morton (akpm@osdl.org) wrote:
>
>
>>"Martin J. Bligh" <mbligh@mbligh.org> wrote:
>>
>>
>>>xen_make_pages_readonly / xen_make_pages_writable ?
>>>
>>>
>>Well we don't want to assume "xen" at this stage. We're faced with a
>>choice at present: to make the linux->hypervisor interface be some
>>xen-specific and xen-controlled thing, or to make it a more formal and
>>controlled kernel interface which talks to a generic hypervisor rather than
>>assuming it's Xen down there.
>>
>>
>
>No, definietly not. Xen is not appropriate global namespace. Also,
>it's not about pages at this point, it's about ldt handling.
>
>
I like these patches. They greatly simplify a lot of the work I had
anticipated was necessary for Xen. I.e. - LDT / GDT accessors are not
needed for most updates, only updates to live descriptor table entries
(for GDT this is TLS, LDT, TSS?, entries and there is 1 LDT update case).
BTW, Martin, did you see my ldt-accessors patch? It also encapsulates
that 1 LDT update case you show here, just named differently.
Yours:
+static inline int install_ldt_entry (__u32 *lp, __u32 entry_1, __u32 entry_2)
+{
+ *lp = entry_1;
+ *(lp+1) = entry_2;
+ return 0;
+}
Mine:
+static inline void write_ldt_entry(void *ldt, int entry, __u32 entry_a, __u32 entry_b)
+{
+ __u32 *lp = (__u32 *)((char *)ldt + entry*8);
+ *lp = entry_a;
+ *(lp+1) = entry_b;
+}
They both work, but mine does not assume page aligned LDTs (necessary to
extract entry number). This is moot right now because LDTs are page
aligned anyway in Linux. I actually don't care which one we use, but it
might be even nicer if we got one with C type checking (struct
desc_struct for ldt).
Does Xen assume page aligned descriptor tables? I assume from this
patch and snippets I have gathered from others, that is a yes, and other
things here imply that DT pages are not shadowed. If so, Xen itself
must have live segments in the GDT pages, so how do you allocate space
for the per-CPU GDT pages on SMP?
>>As long as it doesn't hamper performance or general code sanity, I think it
>>would be better to make this a well-defined and controlled Linux interface.
>>Some of the code to do that is starting to accumulate in -mm. Everyone
>>needs to sit down, take a look at the patches and the proposal and work out
>>if this is the way to proceed.
>>
>>
>
>We're doing that, but it's splintered and coming in from different angles.
>It'd be better to get the story straight then submit patches, IMO.
>
>
I think introducing mach-xen headers is a bit premature though - lets
get the interface nailed down first so that the hypervisor developers
have time to settle the include/asm-i386/mach-xxx files without dealing
unneeded churn onto the maintainers.
Zach
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-08 1:04 ` Zachary Amsden
@ 2005-08-08 1:08 ` Chris Wright
2005-08-08 1:16 ` Zachary Amsden
2005-08-08 1:21 ` Martin J. Bligh
1 sibling, 1 reply; 15+ messages in thread
From: Chris Wright @ 2005-08-08 1:08 UTC (permalink / raw)
To: Zachary Amsden
Cc: Chris Wright, Andrew Morton, Martin J. Bligh, linux-kernel,
Pratap Subrahmanyam
* Zachary Amsden (zach@vmware.com) wrote:
> Does Xen assume page aligned descriptor tables? I assume from this
Yes.
> patch and snippets I have gathered from others, that is a yes, and other
> things here imply that DT pages are not shadowed. If so, Xen itself
> must have live segments in the GDT pages, so how do you allocate space
> for the per-CPU GDT pages on SMP?
early during boot.
> I think introducing mach-xen headers is a bit premature though - lets
> get the interface nailed down first so that the hypervisor developers
> have time to settle the include/asm-i386/mach-xxx files without dealing
> unneeded churn onto the maintainers.
I don't think there's any point in putting mach-xen stuff in either.
Can we please discuss these patches in some sane way (say on the
virutalization list) so we're not cross-firing at each other. It's just
going to be too messy otherwiswe.
thanks,
-chris
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-08 1:08 ` Chris Wright
@ 2005-08-08 1:16 ` Zachary Amsden
2005-08-08 1:36 ` Chris Wright
0 siblings, 1 reply; 15+ messages in thread
From: Zachary Amsden @ 2005-08-08 1:16 UTC (permalink / raw)
To: Chris Wright
Cc: Andrew Morton, Martin J. Bligh, linux-kernel, Pratap Subrahmanyam,
virtualization
Chris Wright wrote:
>* Zachary Amsden (zach@vmware.com) wrote:
>
>
>>Does Xen assume page aligned descriptor tables? I assume from this
>>
>>
>
>Yes.
>
>
>
>>patch and snippets I have gathered from others, that is a yes, and other
>>things here imply that DT pages are not shadowed. If so, Xen itself
>>must have live segments in the GDT pages, so how do you allocate space
>>for the per-CPU GDT pages on SMP?
>>
>>
>
>early during boot.
>
>
Doesn't that require 16 pages per CPU? That seems excessive to impose
on a native build. Perhaps we could get away with 1 page per CPU for
the GDT on native boots and bump that up to 16 if compiling for a
virtualized sub-architecture - i.e. move GDT to a page aligned struct
for native (doesn't cost too much), and give it MACH_GDT_PAGES of space
which is defined by the sub-architecture.
Let's take this thread over to virtualization@lists.osdl.org as well.
Zach
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-08 1:04 ` Zachary Amsden
2005-08-08 1:08 ` Chris Wright
@ 2005-08-08 1:21 ` Martin J. Bligh
1 sibling, 0 replies; 15+ messages in thread
From: Martin J. Bligh @ 2005-08-08 1:21 UTC (permalink / raw)
To: Zachary Amsden, Chris Wright
Cc: Andrew Morton, linux-kernel, Pratap Subrahmanyam
> I like these patches. They greatly simplify a lot of the work I
> had anticipated was necessary for Xen. I.e. - LDT / GDT accessors
> are not needed for most updates, only updates to live descriptor
> table entries (for GDT this is TLS, LDT, TSS?, entries and there
> is 1 LDT update case).
I'm just trying to get rid of as much code duplication as possible.
> BTW, Martin, did you see my ldt-accessors patch? It also
> encapsulates that 1 LDT update case you show here, just named
> differently.
I was focussing on creating a whole Xen stack before looking at
your stuff much, then seeing what was common between them, as I
think it's a bit hard to read the current Xen code because of the
copied files. Unfortunately, is going to be harder then I thought
to maintain that stack out of tree, so I wanted to shovel out
the basic refactoring stuff. Then the line got a bit blurred.
Humpf. And this is the easy part. Damn it.
Doing the whole thing and comparing is going to be a total PITA.
Perhaps the right thing to do is go one file at a time, and sync
up on that basis.
> Yours:
>
> +static inline int install_ldt_entry (__u32 *lp, __u32 entry_1, __u32 entry_2)
> +{
> + *lp = entry_1;
> + *(lp+1) = entry_2;
> + return 0;
> +}
>
> Mine:
>
> +static inline void write_ldt_entry(void *ldt, int entry, __u32 entry_a, __u32 entry_b)
> +{
> + __u32 *lp = (__u32 *)((char *)ldt + entry*8);
> + *lp = entry_a;
> + *(lp+1) = entry_b;
> +}
>
>
> They both work, but mine does not assume page aligned LDTs (necessary
> to extract entry number). This is moot right now because LDTs are
> page aligned anyway in Linux. I actually don't care which one we
> use, but it might be even nicer if we got one with C type checking
> (struct desc_struct for ldt).
Heh, is similar, considering we're working from completely different
angles. I'm just refactoring the current code without changing it
too much at first, we can make it more robust later. otherwise it's
going to be a pig to review if we mix those up.
> I think introducing mach-xen headers is a bit premature though - lets
> get the interface nailed down first so that the hypervisor developers
> have time to settle the include/asm-i386/mach-xxx files without
> dealing unneeded churn onto the maintainers.
I can easily leave those bits out. There's going to be lots of bits common
with std i386, and bits that are common amongst the hypervisor layers,
then bits that are specific. Hopefully more bits that are common, but
still.
Humpf. I shall go back into my corner and have a rethink. Will read through
your patches some more, then send you some email.
M.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-08 1:16 ` Zachary Amsden
@ 2005-08-08 1:36 ` Chris Wright
0 siblings, 0 replies; 15+ messages in thread
From: Chris Wright @ 2005-08-08 1:36 UTC (permalink / raw)
To: Zachary Amsden
Cc: Chris Wright, Andrew Morton, virtualization, linux-kernel,
Martin J. Bligh
* Zachary Amsden (zach@vmware.com) wrote:
> Doesn't that require 16 pages per CPU? That seems excessive to impose
> on a native build. Perhaps we could get away with 1 page per CPU for
> the GDT on native boots and bump that up to 16 if compiling for a
> virtualized sub-architecture - i.e. move GDT to a page aligned struct
> for native (doesn't cost too much), and give it MACH_GDT_PAGES of space
> which is defined by the sub-architecture.
For Xen, the gdt is one page per cpu (so it's not one page per table
entry).
thanks,
-chris
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-07 23:20 [PATCH] abstract out bits of ldt.c Martin J. Bligh
2005-08-07 23:44 ` Chris Wright
@ 2005-08-08 8:41 ` Dave Hansen
1 sibling, 0 replies; 15+ messages in thread
From: Dave Hansen @ 2005-08-08 8:41 UTC (permalink / raw)
To: Martin J. Bligh; +Cc: Andrew Morton, linux-kernel
On Sun, 2005-08-07 at 16:20 -0700, Martin J. Bligh wrote:
> Starting on the work to merge xen cleanly as a subarch.
> Introduce make_pages_readonly and make_pages_writable where appropriate
> for Xen, defined as a no-op on other subarches. Same for
> add_context_to_unpinned and del_context_from_unpinned.
> Abstract out install_ldt_entry()
...
> cpumask_t mask;
> preempt_disable();
> + make_pages_readonly(pc->ldt, (pc->size * LDT_ENTRY_SIZE) /
> + PAGE_SIZE);
> load_LDT(pc);
> mask = cpumask_of_cpu(smp_processor_id());
> if (!cpus_equal(current->mm->cpu_vm_mask, mask))
> smp_call_function(flush_ldt, NULL, 1, 1);
> preempt_enable();
> #else
> + make_pages_readonly(pc->ldt, (pc->size * LDT_ENTRY_SIZE) /
> + PAGE_SIZE);
You do that (size * LDT_ENTRY_SIZE / PAGE_SIZE) operation an awfully
large number of times. Could you consider introducing a little helper,
say ldt_size_pages()?
Or, could you have a helper like make_ldt_readonly()? You don't have to
export it, just use it in that one file.
> This will do have no effect whatsover on platforms other than xen.
...
> + memset(&mm->context, 0, sizeof(mm->context));
> init_MUTEX(&mm->context.sem);
> - mm->context.size = 0;
Could you please explain what this is for? It doesn't appear to be part
of the abstraction.
Every call path I can see to init_new_context() is immediately preceded
by mm_alloc(), which memsets the entire mm. The context is a direct
member of mm_struct, and should be zeroed along with the mm_alloc()
memset. So, this seems a bit superfluous.
In any future patches that you might post, please do one thing per
patch, it makes them much easier to audit.
-- Dave
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
[not found] ` <20050808095755.23810b15.akpm@osdl.org>
@ 2005-08-09 9:23 ` Ingo Molnar
2005-08-09 9:28 ` Christoph Hellwig
0 siblings, 1 reply; 15+ messages in thread
From: Ingo Molnar @ 2005-08-09 9:23 UTC (permalink / raw)
To: Andrew Morton; +Cc: mbligh, chrisw, linux-kernel, zach, torvalds
* Andrew Morton <akpm@osdl.org> wrote:
> > Furthermore, why should we hamper Xen by going to _any_ sort of
> > "formalized" hypervisor API, when we dont even know what we want, as Xen
> > is pretty much work in progress? And whatever Xen support is exported
> > from the kernel, it should be a _GPL symbol export. We dont want to tie
> > down things like pagetable format or access methods. It's a very much
> > kernel-internal thing.
>
> Well that's the other side of the coin. I told the vmware guys to GPL
> their hypervisor to make this issue go away, but that hasn't happened
> yet ;)
>
> I think we need to deliberately deal with all this purely at the
> technical level and to carefully set aside thoughts such as the above.
> Others would disagree with that.
mine are mostly technical arguments. I just also wanted to vent away
this slowly gathering false notion of building 'interoperability', while
the only apparent goal seems to be to maximize benefits to the closed
hypervisors, while allowing them to not open up their code.
just grep the historic Linux changelogs for contributions from the most
prominent of these closed-source hypervisors. It's quite close to zero.
That's what Linux wins from this one-way relationship. Now the action
was forced by Xen, and it sounds so hypocritical to me to talk about
'interoperability'. Once the goal of having the APIs to pull even with
Xen has been achieved, that prominent closed-source hypervisor vendor
could just as easily go back into 'take from Linux'-only mode, as
they've done for so many years.
> At the technical level, I do think that the kernel->hypervisor
> interface is a brand new and *major* kernel interface. As such, it
> simply seems good design to put some thought and some formality into
> it. If that interface suits more than one paravirtualised hypervisor
> implementation then that's a sign that it has succeeded.
it just wont be done right first time around. It will be like with all
the other APIs we had: they went through dozens of major iterations, and
will go through iterations as the hardware changes and things get
cleaned up.
And yes, i do believe the Xen hypervisor should eventually live within
Linux itself. (i.e. Linux should be extended to offer hypervisor
services. This would enable to share drivers and technologies, etc.) No
specifications, no formal agreements necessary - just hack away and
things will be sorted out as they happen.
and no, it's not like with syscalls, for a number of reasons -
hypervisor/OS-kernel integration is a brand-new thing (at least in the
OSS space).
> The other design approach is to make this interface purely some
> xen-private thing which the Xen guys maintain and the rest of us don't
> worry about much. That's also a legitimate approach, but my current
> thinking is that it's technically not as good. Plus other hypervisor
> development teams may come along and have to graft their stuff onto
> the xen-interface-of-the-day, which isn't good.
having a robust API never hurts, no doubt about that - and Xen has
pretty good hypervisor APIs to begin with. But there is no connection
between formality and a technological sound API - you can have a crap
formal API just as much as you can have a good non-formal API. (e.g. the
current Linux device driver APIs are good non-formal APIs)
in fact, history has shown that formality often _hurts_ the
technological soundness of an API, mostly due to the inflebility and the
inherent lag inserted between design and implementation.
Formalizing an API if Xen asks for it is OK in my book, but formalizing
an API mostly for the sake of bin-only virtualization, under the
pretense of 'interoperability' sounds pretty backwards to me.
Ingo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] abstract out bits of ldt.c
2005-08-09 9:23 ` Ingo Molnar
@ 2005-08-09 9:28 ` Christoph Hellwig
0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2005-08-09 9:28 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Andrew Morton, mbligh, chrisw, linux-kernel, zach, torvalds
On Tue, Aug 09, 2005 at 11:23:18AM +0200, Ingo Molnar wrote:
> mine are mostly technical arguments. I just also wanted to vent away
> this slowly gathering false notion of building 'interoperability', while
> the only apparent goal seems to be to maximize benefits to the closed
> hypervisors, while allowing them to not open up their code.
...
I agree completely with the points you make here, but can't find the mail
you are replying to. Where is it coming from?
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2005-08-09 9:28 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-07 23:20 [PATCH] abstract out bits of ldt.c Martin J. Bligh
2005-08-07 23:44 ` Chris Wright
2005-08-07 23:57 ` Martin J. Bligh
2005-08-07 23:59 ` Chris Wright
2005-08-08 0:41 ` Andrew Morton
2005-08-08 0:46 ` Chris Wright
2005-08-08 1:04 ` Zachary Amsden
2005-08-08 1:08 ` Chris Wright
2005-08-08 1:16 ` Zachary Amsden
2005-08-08 1:36 ` Chris Wright
2005-08-08 1:21 ` Martin J. Bligh
2005-08-08 0:59 ` Martin J. Bligh
[not found] ` <20050808113014.GA15165@elte.hu>
[not found] ` <20050808095755.23810b15.akpm@osdl.org>
2005-08-09 9:23 ` Ingo Molnar
2005-08-09 9:28 ` Christoph Hellwig
2005-08-08 8:41 ` Dave Hansen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox