[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3] x86/hvm/viridian: flush remote tlbs by hypercall
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@xxxxxxxx] > Sent: 20 November 2015 09:45 > To: Andrew Cooper; Paul Durrant > Cc: Ian Campbell; Ian Jackson; Stefano Stabellini; Wei Liu; xen- > devel@xxxxxxxxxxxxxxxxxxxx; Keir (Xen.org) > Subject: RE: [PATCH v3] x86/hvm/viridian: flush remote tlbs by hypercall > > >>> On 20.11.15 at 10:15, <Paul.Durrant@xxxxxxxxxx> wrote: > >> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx] > >> Sent: 19 November 2015 17:09 > >> On 19/11/15 16:57, Paul Durrant wrote: > >> >>> + /* > >> >>> + * For each specified virtual CPU flush all ASIDs to > >> >>> invalidate > >> >>> + * TLB entries the next time it is scheduled and then, if it > >> >>> + * is currently running, add its physical CPU to a mask of > >> >>> + * those which need to be interrupted to force a flush. > >> >>> + */ > >> >>> + for_each_vcpu ( currd, v ) > >> >>> + { > >> >>> + if ( !(input_params.vcpu_mask & (1ul << v->vcpu_id)) ) > >> >>> + continue; > >> >> You need to cap this loop at a vcpu_id of 63, or the above conditional > >> >> will become undefined. > >> > The guest should not issue the hypercall if it has more than 64 vCPUs so > to > >> some extend I don't care what happens, as long as it is not harmful to the > >> hypervisor in general, and I don't think that it is in this case. > >> > >> The compiler is free to do anything it wishes when encountering > >> undefined behaviour, including crash the hypervisor. > >> > > > > I don't believe so. See section 6.5.7 of the C99 spec. (top pf page 85): > > > > "The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits > > are > > filled with > > zeros. If E1 has an unsigned type, the value of the result is E1 x 2^E2, > > reduced modulo > > one more than the maximum value representable in the result type. If E1 > has > > a signed > > type and nonnegative value, and E1 x 2^E2 is representable in the result > > type, then that is > > the resulting value; otherwise, the behavior is undefined" > > > > So, the undefined behaviour you refer to is only in the case that E1 is > > signed and in this case it is most definitely unsigned, so the modulo rule > > applies. > > Looks like you missed the earlier clause 3 (at the bottom of page 84): > "If the value of the right operand is negative or is greater than or > equal to the width of the promoted left operand, the behavior is > undefined." And if you look at generated code, I think you'll find > that the result is not zero for too large shift counts, as would need > to be the case if that clause wasn't there. > Hmm. That does seem contradictory. It does indeed say 'behavior' is undefined. If it was merely 'value' is undefined then I would not see that as a problem. > I'll actually make my previous Reviewed-by dependent on this issue > getting fixed. > Since you've convinced me that the check is necessary, I'll post v5 with it in. Paul > >> Any undefined-behaviour which can be triggered by a guest action > >> warrants an XSA, because there is no telling what might happen. > > Well, it warrants fixing, but I don't think it unconditionally would > be XSA-worthy. After all, the compiler (normally) does sane > things in response - in order to crash the hypervisor or produce > truly undefined behavior, it would need to generate extra code > (since other than in C, at the assembly level the instruction's > behavior is defined), which I think we can take for granted it > won't. > > Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |