Re: [PATCH v4 4/7] xen, common: add the XEN_DOMCTL_memory_mapping hypercall

[Arianna Avanzini <avanzini.arianna@gmail.com>]

On 04/02/2014 12:53 PM, Jan Beulich wrote:
>>>> On 02.04.14 at 12:19, <Ian.Campbell@eu.citrix.com> wrote:
>> On Wed, 2014-04-02 at 11:06 +0100, Jan Beulich wrote:
>>>>>> On 02.04.14 at 11:43, <Ian.Campbell@eu.citrix.com> wrote:
>>>> This interface is already pretty broken for the case where a toolstack
>>>> maps the same mfns into a guest twice. But that is a pretty broken thing
>>>> for a toolstack to do.
>>>
>>> Is it? In the context of the VGA ROM issue in qemu (discussed recently)
>>> it seemed reasonable to me to map this at both the PCI BAR specified
>>> location and the legacy ISA C0000h one.
>>
>> That's emulated though isn't it? This interface is about real machine
>> iomem pages.
> 
> Could be either (VGA passthrough, or PXE boot ROM on a passed
> through NIC), at least in theory.
> 
>> Anyway, the issue is that the permission is not reference counted so you
>> can do:
>>         map mfn 0x12345 at gfn 0xc0000
>>         map mfn 0x12345 at gfn 0xf0000000
>>         unmap mfn 0x12345 from 0xf0000000
>>         
>> Now the guest no longer has permission (as in iomem_access_permitted) to
>> access mfn 0x12345 but it still has a mapping at gfn 0xc0000 and can use
>> it (unless perhaps some iommu stuff goes on which I haven't grokked).
> 
> That's ugly indeed.
> 
>> Perhaps we don't care about that and expect the toolstack/device model
>> to ensure that it unmaps everything or nothing. (I didn't check if a
>> subsequent unmap from 0xc0000 would work BTW).
> 
> In fact we wouldn't, at the example of the ROMs again: The mapping
> in the legacy ISA area should always be there, whereas the one at
> the PCI BAR specified location should only be there if the ROM is
> enabled.
> 
>> If we do care about this issue then our choices are:
>>       * Implement proper referencing counting of iomem permissions
>>         (tricky?)
>>       * Force the reference counting to be trivial by requiring that the
>>         reference is at most 1
>>       * Split the manipulation of iomem permissions out of
>>         XEN_DOMCTL_memory_mapping and require the toolstack to issue
>>         XEN_DOMCTL_iomem_permission first (and last after all mappings
>>         are removed).
> 
> This one seems the only reasonable approach to me. Yet in order to
> prevent the (disaggregated) tool stack from revoking the permission
> without removing the mapping, some reference counting would still
> be needed. Or maybe we don't need to care about that case, as long
> as the memory range can't be assigned to another guest not
> controlled by the same tool stack instance.
> 
>>       * Do some sort of exhaustive search on unmap like Julien suggested
> 
> Wrt unmapping, I think the MFN mapped at the specified GFN
> shouldn't really matter - if the tool stack says "unmap", so be it.
> And hence there's nothing really to search for.
> 

Hello,

sorry if I intrude with a question. I have been working on a new version of the
patch series, just to try to get out of the way the most immediate issues that
everybody pointed out.

Is everybody OK if I send it or would you prefer me to wait for a decision in
this new thread of discussion and try to include in the next patchset something
related to it? I'm obviously fine either way.

Thank you for all your comments and, again, sorry for the question.



-- 
/*
 * Arianna Avanzini
 * avanzini.arianna@gmail.com
 * 73628@studenti.unimore.it
 */