Re: [Xen-devel] [PATCH v5 05/15] x86/altp2m: basic data structures and support routines.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.07.15 at 00:36, <ravi.sahita@xxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>Sent: Thursday, July 16, 2015 2:08 AM
>>
>>>>> On 16.07.15 at 10:57, <ravi.sahita@xxxxxxxxx> wrote:
>>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>>>Sent: Tuesday, July 14, 2015 6:13 AM
>>>>>>> On 14.07.15 at 02:14, <edmund.h.white@xxxxxxxxx> wrote:
>>>>> @@ -722,6 +731,27 @@ void nestedp2m_write_p2m_entry(struct
>>>>p2m_domain *p2m, unsigned long gfn,
>>>>>      l1_pgentry_t *p, l1_pgentry_t new, unsigned int level);
>>>>>
>>>>>  /*
>>>>> + * Alternate p2m: shadow p2m tables used for alternate memory views
>>>>> + */
>>>>> +
>>>>> +/* get current alternate p2m table */ static inline struct
>>>>> +p2m_domain *p2m_get_altp2m(struct vcpu *v) {
>>>>> +    struct domain *d = v->domain;
>>>>> +    uint16_t index = vcpu_altp2m(v).p2midx;
>>>>> +
>>>>> +    ASSERT(index < MAX_ALTP2M);
>>>>> +
>>>>> +    return (index == INVALID_ALTP2M) ? NULL :
>>>>> +d->arch.altp2m_p2m[index]; }
>>>>
>>>>Looking at this again, I'm afraid I'd still prefer index < MAX_ALTP2M
>>>>in the return statement (and the ASSERT() dropped): The ASSERT() does
>>>>nothing in a debug=n build, and hence wouldn't shield us from possibly
>>>>having to issue an XSA if somehow an access outside the array's bounds
>>turned out possible.
>>>>
>>>
>>> the assert was breaking v5 anyway. BUG_ON (with the right check) is
>>> probably the right thing to do, as we do in the exit handling that
>>> checks for a VMFUNC having changed the index.
>>> So will make that change.
>>
>>But why use a BUG_ON() when you can deal with this more gracefully? Please
>>try to avoid crashing the hypervisor when there are other ways to recover.
>>
> 
> So in this case there isnt a graceful fallback; this case can happen only if 
> there is a bug in the hypervisor - which should be reported via the BUG_ON.

Generally (as an example), if a hypervisor bug can be confined to a
guest, killing just the guest instead of the hypervisor would still be
preferred (allowing the admin to attempt to gracefully shut down
other guests before updating/restarting).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel