Re: [Xen-devel] [PATCH v1] x86/altp2m: Add hypercall to create a new view and set sve bits

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.09.2019 16:14, Alexandru Stefan ISAILA wrote:
> 
> 
> On 04.09.2019 16:17, Jan Beulich wrote:
>> On 04.09.2019 15:04, Alexandru Stefan ISAILA wrote:
>>>
>>>
>>> On 04.09.2019 15:14, Jan Beulich wrote:
>>>> On 04.09.2019 13:51, Alexandru Stefan ISAILA wrote:
>>>>>
>>>>>
>>>>> On 03.09.2019 18:52, Jan Beulich wrote:
>>>>>> On 02.09.2019 10:11, Alexandru Stefan ISAILA wrote:
>>>>>>> @@ -1355,6 +1355,23 @@ void p2m_init_altp2m_ept(struct domain *d, 
>>>>>>> unsigned int i)
>>>>>>>         ept = &p2m->ept;
>>>>>>>         ept->mfn = pagetable_get_pfn(p2m_get_pagetable(p2m));
>>>>>>>         d->arch.altp2m_eptp[i] = ept->eptp;
>>>>>>> +
>>>>>>> +    if ( set_sve )
>>>>>>> +    {
>>>>>>> +        unsigned long gfn = 0, max_gpfn = domain_get_maximum_gpfn(d);
>>>>>>> +
>>>>>>> +        for( ; gfn < max_gpfn; ++gfn )
>>>>>>> +        {
>>>>>>> +            mfn_t mfn;
>>>>>>> +            p2m_access_t a;
>>>>>>> +            p2m_type_t t;
>>>>>>> +
>>>>>>> +            altp2m_get_effective_entry(p2m, _gfn(gfn), &mfn, &t, &a,
>>>>>>> +                                       AP2MGET_query);
>>>>>>> +            p2m->set_entry(p2m, _gfn(gfn), mfn, PAGE_ORDER_4K, t, a, 
>>>>>>> true);
>>>>>>> +
>>>>>>> +        }
>>>>>>> +    }
>>>>>>>     }
>>>>>>
>>>>>> How long is this loop going to take for a huge guest? IOW how
>>>>>> come there's no preemption in here, or some other mechanism
>>>>>> to bound execution time?
>>>>>
>>>>> Because this is done for the initialization of a new view and the p2m is
>>>>> locked.
>>>>
>>>> Well, this makes handling this the way you want it close to
>>>> impossible, but is not an argument against the need for preemption
>>>> here. Just like it had turned out to be unreasonable to
>>>> preemptively handle other P2M adjustments (which is why
>>>> p2m-ept.c:resolve_misconfig() and p2m-pt.c:do_recalc() got
>>>> introduced), I'm afraid you'll have to use some other technique
>>>> here (possibly building on top of the mentioned functions).
>>>>
>>>
>>> I think that the mechanism from p2m_set_mem_access_multi() can suit this
>>> case, start the loop, set ,if(hypercall_preempt_check()) rc =
>>> next_unset_gfn;
>>>
>>> And for this to work it should have a new "start_gfn" parameter so the
>>> caller can issue multiple hypercalls until gfn == max_gfn.
>>
>> Hmm, possible. I took your previous reply to mean that it is
>> important for the p2m to not get unlocked in the middle of this
>> process. If this was a wrong understanding of mine, then yes,
>> "conventional" preemption like you outline it ought to work.
>>
> 
> There are two possible ways to solve this:
> 1. the conventional preemption way and with this I do not return ok on 
> the init function until the final gfn has the bit set (code will have to 
> change to accommodate this new option/preemption).
> 
> 2. as you described and work with p2m->recalc() (I will have to check 
> this out to have a correct idea of what is to be done).
> 
> What do you think is the preferred way to continue with this?

Unless I'm missing information on the requirements I think either
would be fine. If the change was much simpler using the recalc
code, perhaps that'd be the way to go. In all other cases I'd
suggest to try to leave the already non-trivial recalc code alone.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel