Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [PATCH 2 of 7] x86/pvops: add a paravirt_ident functions to allow special patching | Date | Thu, 29 Jan 2009 18:35:48 +1030 |
| |
On Thursday 29 January 2009 09:05:02 Jeremy Fitzhardinge wrote: > void _paravirt_nop(void); > +u32 _paravirt_ident_32(u32); > +u64 _paravirt_ident_64(u64); > + > #define paravirt_nop ((void *)_paravirt_nop)
So, we used a void * cast for the paravirt_nop case, but you decided to use explicit types for the ident cases?
> if (opfunc == NULL) > /* If there's no function, patch it with a ud2a (BUG) */ > ret = paravirt_patch_insns(insnbuf, len, ud2a, ud2a+sizeof(ud2a)); > - else if (opfunc == paravirt_nop) > + else if (opfunc == _paravirt_nop) > /* If the operation is a nop, then nop the callsite */ > ret = paravirt_patch_nop();
Gratuitous change?
> +typedef pte_t make_pte_t(pteval_t); > +typedef pmd_t make_pmd_t(pmdval_t); > +typedef pud_t make_pud_t(pudval_t); > +typedef pgd_t make_pgd_t(pgdval_t); > + > +typedef pteval_t pte_val_t(pte_t); > +typedef pmdval_t pmd_val_t(pmd_t); > +typedef pudval_t pud_val_t(pud_t); > +typedef pgdval_t pgd_val_t(pgd_t); > + > + > +#if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE) > +/* 32-bit pagetable entries */ > +#define paravirt_native_make_pte (make_pte_t *)_paravirt_ident_32 > +#define paravirt_native_pte_val (pte_val_t *)_paravirt_ident_32 > + > +#define paravirt_native_make_pmd (make_pmd_t *)_paravirt_ident_32 > +#define paravirt_native_pmd_val (pmd_val_t *)_paravirt_ident_32 > + > +#define paravirt_native_make_pud (make_pud_t *)_paravirt_ident_32 > +#define paravirt_native_pud_val (pud_val_t *)_paravirt_ident_32 > + > +#define paravirt_native_make_pgd (make_pgd_t *)_paravirt_ident_32 > +#define paravirt_native_pgd_val (pgd_val_t *)_paravirt_ident_32 > +#else > +/* 64-bit pagetable entries */ > +#define paravirt_native_make_pte (make_pte_t *)_paravirt_ident_64 > +#define paravirt_native_pte_val (pte_val_t *)_paravirt_ident_64 > + > +#define paravirt_native_make_pmd (make_pmd_t *)_paravirt_ident_64 > +#define paravirt_native_pmd_val (pmd_val_t *)_paravirt_ident_64 > + > +#define paravirt_native_make_pud (make_pud_t *)_paravirt_ident_64 > +#define paravirt_native_pud_val (pud_val_t *)_paravirt_ident_64 > + > +#define paravirt_native_make_pgd (make_pgd_t *)_paravirt_ident_64 > +#define paravirt_native_pgd_val (pgd_val_t *)_paravirt_ident_64 > +#endif
I think I prefer:
/* make_pte etc and pgd_val etc are identity functions. */ #define paravirt_native_page_op \ (sizeof(pte_t) == sizeof(u64) ? paravirt_ident_64 : paravirt_ident_32)
Then use that everywhere rather than these defines?
But it's a minor point; the code seems perfectly sound.
Acked-by: Rusty Russell <rusty@rustcorp.com.au>
Thanks! Rusty.
| |