Re: [PATCH] x86: refine guest_mode()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, May 22, 2020 at 02:00:22PM +0200, Jan Beulich wrote:
> On 22.05.2020 12:48, Roger Pau Monné wrote:
> > On Fri, May 22, 2020 at 11:52:42AM +0200, Jan Beulich wrote:
> >> On 20.05.2020 17:13, Roger Pau Monné wrote:
> >>> OK, so I think I'm starting to understand this all. Sorry it's taken
> >>> me so long. So it's my understanding that diff != 0 can only happen in
> >>> Xen context, or when in an IST that has a different stack (ie: MCE, NMI
> >>> or DF according to current.h) and running in PV mode?
> >>>
> >>> Wouldn't in then be fine to use (r)->cs & 3 to check we are in guest
> >>> mode if diff != 0? I see a lot of other places where cs & 3 is already
> >>> used to that effect AFAICT (like entry.S).
> >>
> >> Technically this would be correct afaics, but the idea with all this
> >> is (or should I say "looks to be"?) to have the checks be as tight as
> >> possible, to make sure we don't mistakenly consider something "guest
> >> mode" which really isn't. IOW your suggestion would be fine with me
> >> if we could exclude bugs anywhere in the code. But since this isn't
> >> realistic, I consider your suggestion to be relaxing things by too
> >> much.
> > 
> > OK, so I take that (long time) we might also want to change the cs & 3
> > checks from entry.S to check against __HYPERVISOR_CS explicitly?
> 
> I didn't think so, no (not the least because of there not being any
> guarantee afaik that EFI runtime calls couldn't play with segment
> registers; they shouldn't, yes, but there's a lot of other "should"
> many don't obey to). Those are guaranteed PV-only code paths. The
> main issue here is that ->cs cannot be relied upon when a frame
> points at HVM state.

Well, if it points at HVM state it could equally have __HYPERVISOR_CS
set by the guest.

Will things work anyway if you get here from an exception generated by
EFI code that has changed the code segment? You are going to hit the
assert at least, since diff will be != 0 and cs != __HYPERVISOR_CS?

I would prefer to keep things coherent by either using cs & 3 or
cs == __HYPERVISOR_CS everywhere if possible, as I'm still unsure of
the benefit of using __HYPERVISOR_CS.

Thanks, Roger.