Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [PATCH RFC REPOST 1/2] paravirt: refactor struct paravirt_ops into smaller pv_*_ops | Date | Wed, 10 Oct 2007 16:35:09 +1000 |
| |
On Wednesday 10 October 2007 04:24:24 Jeremy Fitzhardinge wrote: > [ I think this is a straight repost this patch, which addresses all the > previous comments. I'd like to submit this for .24 as the basis for a > unified paravirt_ops. Any objections? ]
Thanks Jeremy, I've actually taken time to finally review this in detail (I'm assuming you'll refactor as necessary after the x86 arch merger).
> + OFFSET(PARAVIRT_enabled, pv_info, paravirt_enabled);
I think this gives the right answer for the wrong reasons?
> +struct paravirt_ops paravirt_ops; > +
Do you actually need to define this? See below...
> +DEF_NATIVE(, ud2a, "ud2a");
Hmm, that's ugly. It was ugly before, but it's uglier now. Maybe just use "unsigned char ud2a[] = { 0x0f, 0x0b };" in paravirt_patch_default?
> } > > struct paravirt_ops paravirt_ops = { ... > + .pv_info = { > + .name = "bare hardware", > + .paravirt_enabled = 0, > + .kernel_rpl = 0, > + .shared_kernel_pmd = 1, /* Only used when CONFIG_X86_PAE is set */ > + },
This is the bit I don't get. Why not just declare struct pv_info pvinfo, etc, and use the declaration of struct paravirt_ops to get your unique offset-based identifiers for patching?
Rest looks fine...
Thanks! Rusty. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |