Re: [Xen-devel] [PATCH] x86/altp2m: Add a subop for obtaining the mem access of a page

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Jul 4, 2018, at 4:38 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> 
>>>> On 04.07.18 at 16:05, <George.Dunlap@xxxxxxxxxx> wrote:
>>> On Jul 2, 2018, at 7:34 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>> On 29.06.18 at 18:39, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>> On 06/29/2018 06:38 PM, Jan Beulich wrote:
>>>>>>>> On 28.06.18 at 15:00, <apop@xxxxxxxxxxxxxxx> wrote:
>>>>>> @@ -4666,6 +4667,23 @@ static int do_altp2m_op(
>>>>>>        }
>>>>>>        break;
>>>>>> 
>>>>>> +    case HVMOP_altp2m_get_mem_access:
>>>>>> +        if ( a.u.mem_access.pad )
>>>>>> +            rc = -EINVAL;
>>>>>> +        else
>>>>>> +        {
>>>>>> +            xenmem_access_t access;
>>>>>> +
>>>>>> +            rc = p2m_get_mem_access(d, _gfn(a.u.mem_access.gfn), 
>>>>>> &access,
>>>>>> +                                    a.u.mem_access.view);
>>>>>> +            if ( !rc )
>>>>>> +            {
>>>>>> +                a.u.mem_access.hvmmem_access = access;
>>>>>> +                rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
>>>>> 
>>>>> __copy_field_to_guest()? Or wait, no, the function argument is still a
>>>>> handle of void.
>>>>> 
>>>>> And then - here we are again: Is it reasonable to permit a domain 
>>>>> inquiring
>>>>> for itself?
>>>> 
>>>> A good question. Perhaps the following are decision factors:
>>>> 
>>>> 1. It is already possible for a domain to set mem_access restrictions
>>>> (via HVMOP_altp2m_set_mem_access) on itself.
>>> 
>>> Which, as before, I consider a flaw.
>> 
>> How many times do we have to go over this?  Here is my recollection from the 
>> last time we had a discussion on this topic:
>> 
>> * The original authors of this code probably thought having guests set their 
>> own memaccess would be a potential use case
>> * The maintainers and main users of the code (Tamas and Razvan) think it’s a 
>> useful use case
>> * The MM maintainer (me) and one of the x86 maintainers (Andy) think it’s a 
>> useful use case.
>> 
>> (Correct me if I’ve misremembered anywhere.)
>> 
>> Do we need to have a formal vote by the committers for you to accept that 
>> this should be a supported use case, and stop making objections any time 
>> someone wants to improve it?
> 
> There's no need for a vote, since - as before - I won't object to the
> addition, but I consider it to widen the badness (once again).

But you did object.  This whole thread is you re-objecting to the original 
decision that we’ve discussed twice before.  Either the altp2m functionality 
should be exposed to the guest, or it shouldn’t.  If we do expose functionality 
to the guest, then the interface exposed should be useful; and being able to 
read what you wrote, rather than keeping a separate copy of it, is part of a 
useful interface.

I mean, I understand objecting to the idea of building an extension to your 
house.  But what doesn’t make sense to me is, once the extension is built, then 
objecting to the idea of painting it; and then objecting to the idea of adding 
electrical sockets; and then objecting to the idea of adding heat.  Why not 
just accept that you have an extra room and make the best of it?  I understand 
that you’d prefer extra garden space to the utility room that’s there now, but 
given that you can’t have the garden, why is a utility room with no paint and 
no electricity and no heat better than a utility room with all those things?

> In all
> the "think it's a valid use case" it was never really made clear to me
> how this "valid" implies "still secure”.

That was never the argument.  The argument is that the behavior is off by 
default, and that host administrators should be treated as adults and allowed 
to judge for themselves whether it’s safe to turn it on or not — just like 
nested virt, PCI pass-through, COLO, or the host of other features that aren’t 
security supported.

I mean, I’d understand if supporting that use case this meant add tons of extra 
functionality that was likely to be fragile and introduce new bugs; but it’s 
not — all the complexity of memaccess would be there even if we only allowed 
dom0 access to this functionality.

Would you feel better if we had a line covering memaccess in SUPPORT.md?

 -George


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