Re: [Xen-devel] [PATCH v4 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.06.16 at 09:45, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
> On 6/20/2016 9:38 PM, Jan Beulich wrote:
>>>>> On 20.06.16 at 14:06, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
>>> However, if live migration is started(all pte entries invalidated
>>> again), resolve_misconfig() would
>>> change both gfn A's and gfn B's p2m type back to p2m_ram_rw, which means
>>> the emulation of
>>> gfn B would fail.
>> Why would it? Changes to p2m_ram_logdirty won't alter
>> p2m_ioreq_server entries, and hence changes from it back to
>> p2m_ram_rw won't either.
> 
> Oh, above example is based on the assumption that resolve_misconfig() is 
> extended
> to handle the p2m_ioreq_server case(see my "Suppose resolve_misconfig() 
> is modified...").
> The code change could be something like below:
> 
> @@ -542,10 +542,14 @@ static int resolve_misconfig(struct p2m_domain 
> *p2m, unsigned long gfn)
> 
> -                    if ( e.recalc && p2m_is_changeable(e.sa_p2mt) )
> +                   if ( e.recalc )
>                       {
> -                         e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn + 
> i, gfn + i)
> -                                     ? p2m_ram_logdirty : p2m_ram_rw;
> +                         if ( e.sa_p2mt == p2m_ioreq_server )
> +                             e.sa_p2mt = p2m_ram_rw;
> +                         else if ( p2m_is_changeable(e.sa_p2mt) )
> +                             e.sa_p2mt = p2m_is_logdirty_range(p2m, gfn 
> + i, gfn + i)
> +                                         ? p2m_ram_logdirty : p2m_ram_rw;
> +
>                            ept_p2m_type_to_flags(p2m, &e, e.sa_p2mt, 
> e.access);
>                       }
>                       e.recalc = 0;
> 
> With changes like this, both p2m types of gfn A and gfn B from above example
> would be set to p2m_ram_rw if log dirty is enabled.

Above modification would convert _all_ p2m_ioreq_server into
p2m_ram_rw, irrespective of log-dirty mode being active. Which
I don't think is what you want.

> So that's what I am worrying - if a user unintentionally typed "xl save" 
> during
> the emulation process , the emulation would fail. We can let the 
> enable_logdirty()
> return false if XenGT is detected, but we still wish to keep the log 
> dirty feature.

Well, enabling log-dirty mode would succeed as soon as all
p2m_ioreq_server pages got converted back to normal ones (by
the device model). So an unintentional "xl save" would simply fail.
Is there any problem with that?

>> And then - didn't we mean to disable that part of XenGT during
>> migration, i.e. temporarily accept the higher performance
>> overhead without the p2m_ioreq_server entries? In which case
>> flipping everything back to p2m_ram_rw after (completed or
>> canceled) migration would be exactly what we want. The (new
>> or previous) ioreq server should attach only afterwards, and
>> can then freely re-establish any p2m_ioreq_server entries it
>> deems necessary.
>>
> 
> Well, I agree this part of XenGT should be disabled during migration. 
> But in such
> case I think it's device model's job to trigger the p2m type 
> flipping(i.e. by calling
> HVMOP_set_mem_type).

I agree - this would seem to be the simpler model here, despite (as
George validly says) the more consistent model would be for the
hypervisor to do the cleanup. Such cleanup would imo be reasonable
only if there was an easy way for the hypervisor to enumerate all
p2m_ioreq_server pages.

> And the device model should be notified first when the
> migration begins - we may need new patches to do so if XenGT is going to 
> support
> vGPU migration in the future.

Quite possible.

Jan


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