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

[Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>]

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?

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