Messages in this thread | | | Date | Tue, 4 Oct 2022 17:33:58 +0200 | Subject | Re: [PATCH v2 2/3] xen/pv: refactor msr access functions to support safe and unsafe accesses | From | Juergen Gross <> |
| |
On 04.10.22 13:03, Jan Beulich wrote: > On 04.10.2022 10:43, Juergen Gross wrote: >> Refactor and rename xen_read_msr_safe() and xen_write_msr_safe() to >> support both cases of MSR accesses, safe ones and potentially GP-fault >> generating ones. >> >> This will prepare to no longer swallow GPs silently in xen_read_msr() >> and xen_write_msr(). >> >> Signed-off-by: Juergen Gross <jgross@suse.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Albeit ... > >> @@ -933,23 +937,39 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err) >> return val; >> } >> >> -static int xen_write_msr_safe(unsigned int msr, unsigned low, unsigned high) >> +static void set_seg(unsigned int which, unsigned int low, unsigned int high, >> + int *err) >> { >> - int ret; >> - unsigned int which; >> - u64 base; >> + u64 base = ((u64)high << 32) | low; >> + >> + if (HYPERVISOR_set_segment_base(which, base) == 0) >> + return; >> >> - ret = 0; >> + if (err) >> + *err = -EIO; > > ... I don't see a good reason to override the error code handed to us > by the hypervisor here; I do realize though that this ... > >> + else >> + WARN(1, "Xen set_segment_base(%u, %llx) failed\n", which, base); >> +} >> >> +/* >> + * Support write_msr_safe() and write_msr() semantics. >> + * With err == NULL write_msr() semantics are selected. >> + * Supplying an err pointer requires err to be pre-initialized with 0. >> + */ >> +static void xen_do_write_msr(unsigned int msr, unsigned int low, >> + unsigned int high, int *err) >> +{ >> switch (msr) { >> - case MSR_FS_BASE: which = SEGBASE_FS; goto set; >> - case MSR_KERNEL_GS_BASE: which = SEGBASE_GS_USER; goto set; >> - case MSR_GS_BASE: which = SEGBASE_GS_KERNEL; goto set; >> - >> - set: >> - base = ((u64)high << 32) | low; >> - if (HYPERVISOR_set_segment_base(which, base) != 0) >> - ret = -EIO; > > ... was this way before.
And on bare metal write_msr_safe() will return -EIO, too.
Juergen [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |