On Thu, 2022-10-20 at 18:37 +0000, Sean Christopherson wrote:
> On Thu, Oct 20, 2022, Maxim Levitsky wrote:
>
> Changelog please.
> > Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
> > ---
> > lib/x86/svm_lib.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
> > lib/x86/svm_lib.h | 4 ++++
>
> What about calling these simply svm.{c,h} and renaming x86/svm.{c,h} to something
> like svm_common.{c,h}? Long term, it would be wonderful to rid of x86/svm.{c,h}
> by genericizing the test framework, e.g. there's a ton of duplicate code between
> SVM and VMX.
Makes sense.
>
> > x86/svm.c | 54 -----------------------------------------------
> > x86/svm.h | 1 -
> > 4 files changed, 58 insertions(+), 55 deletions(-)
> >
> > diff --git a/lib/x86/svm_lib.c b/lib/x86/svm_lib.c
> > index 9e82e363..2b067c65 100644
> > --- a/lib/x86/svm_lib.c
> > +++ b/lib/x86/svm_lib.c
> > @@ -103,3 +103,57 @@ void setup_svm(void)
> >
> > setup_npt();
> > }
> > +
> > +void vmcb_set_seg(struct vmcb_seg *seg, u16 selector,
> > + u64 base, u32 limit, u32 attr)
>
> Funky indentation and wrap.
>
> void vmcb_set_seg(struct vmcb_seg *seg, u16 selector, u64 base, u32 limit,
> u32 attr)
>
> > +{
> > + seg->selector = selector;
> > + seg->attrib = attr;
> > + seg->limit = limit;
> > + seg->base = base;
> > +}
> > +
> > +void vmcb_ident(struct vmcb *vmcb)
> > +{
> > + u64 vmcb_phys = virt_to_phys(vmcb);
> > + struct vmcb_save_area *save = &vmcb->save;
> > + struct vmcb_control_area *ctrl = &vmcb->control;
> > + u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK
>
> Ugh, a #define for '3' and '9' (in lib/x86/desc.h?) would be nice, but that can
> be left for another day/patch.
Exactly.
>
> > + | SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK;
>
> Pre-existing mess, but can you move the '|' to the previous line? And align the
> code?
>
> > + u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK
> > + | SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK;
>
> > on the previous line.
OK.
>
> u32 data_seg_attr = 3 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK |
> SVM_SELECTOR_DB_MASK | SVM_SELECTOR_G_MASK;
> u32 code_seg_attr = 9 | SVM_SELECTOR_S_MASK | SVM_SELECTOR_P_MASK |
> SVM_SELECTOR_L_MASK | SVM_SELECTOR_G_MASK;
>
Best regards,
Maxim Levitsky
