Re: [PATCH v10 5/6] x86/ioreq server: Asynchronously reset outstanding p2m_ioreq_server entries.

[George Dunlap <george.dunlap@citrix.com>]

On 05/04/17 17:22, Yu Zhang wrote:
> 
> 
> On 4/5/2017 10:41 PM, George Dunlap wrote:
>> On Sun, Apr 2, 2017 at 1:24 PM, Yu Zhang <yu.c.zhang@linux.intel.com>
>> wrote:
>>> After an ioreq server has unmapped, the remaining p2m_ioreq_server
>>> entries need to be reset back to p2m_ram_rw. This patch does this
>>> asynchronously with the current p2m_change_entry_type_global()
>>> interface.
>>>
>>> New field entry_count is introduced in struct p2m_domain, to record
>>> the number of p2m_ioreq_server p2m page table entries. One nature of
>>> these entries is that they only point to 4K sized page frames, because
>>> all p2m_ioreq_server entries are originated from p2m_ram_rw ones in
>>> p2m_change_type_one(). We do not need to worry about the counting for
>>> 2M/1G sized pages.
>> Assuming that all p2m_ioreq_server entries are *created* by
>> p2m_change_type_one() may valid, but can you assume that they are only
>> ever *removed* by p2m_change_type_one() (or recalculation)?
>>
>> What happens, for instance, if a guest balloons out one of the ram
>> pages?  I don't immediately see anything preventing a p2m_ioreq_server
>> page from being ballooned out, nor anything on the
>> decrease_reservation() path decreasing p2m->ioreq.entry_count.  Or did
>> I miss something?
>>
>> Other than that, only one minor comment...
> 
> Thanks for your thorough consideration, George. But I do not think we
> need to worry about this:
> 
> If the emulation is in process, the balloon driver cannot get a
> p2m_ioreq_server page - because
> it is already allocated.

In theory, yes, the guest *shouldn't* do this.  But what if the guest OS
makes a mistake?  Or, what if the ioreq server makes a mistake and
places a watch on a page that *isn't* allocated by the device driver, or
forgets to change a page type back to ram when the device driver frees
it back to the guest kernel?

It's the hypervisor's job to do the right thing even if the guest and
the device model don't.

> And even when emulation is finished, the balloon driver successfully get
> this page, and triggers
> decrease_reservation, the purpose is to remove the current mapping
> relation between the gfn
> and mfn in p2m. So IIUC, p2m_remove_page() will be triggered if
> everything is goes fine, and then
> p2m_set_entry(), which will trigger the recalc logic eventually, either
> in ept_set_entry() or
> p2m_pt_set_entry(). Then the entry_count will be updated in the recalc
> logic.

Yes, once the lazy type change has been made, we can rely on the
recalculation logic to make sure that the types are changed appropriately.

 -George


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