Re: [Xen-devel] [v2 06/11] vmx: add help functions to support PML

[Kai Huang <kai.huang@xxxxxxxxxxxxxxx>]

On 04/17/2015 08:10 AM, Tim Deegan wrote:
> At 22:57 +0000 on 16 Apr (1429225024), Tian, Kevin wrote:
>
> > > From: Kai Huang [mailto:kai.huang@xxxxxxxxxxxxxxx]
> > > +        if ( !p2m_change_type_one(v->domain, gfn, p2m_ram_logdirty,
> > > +                    p2m_ram_rw) )
> > > +            paging_mark_gfn_dirty(v->domain, gfn);
> > Should we handle error from p2m_change_type_one and consequently
> > making this flush function non-void?
> I don't think we need to return an error, but we should probably
> call mark_dirty here for anything except -EBUSY.
Hi Kevin, Tim,

My intention here is to rule out the GFN with original type that is not 
p2m_ram_logdirty, though with patch 11 it's not likely have such GFN 
logged.
Looks -EBUSY returns exactly when original type is not p2m_ram_logdirty, 
so I think it might be OK to do as Tim suggested.
But given the same thing has already been done in hap_track_dirty_vram 
(hap_track_dirty_vram->paging_log_dirty_range, in which gfn is set in 
bitmap with !p2m_change_type_one is true), and in EPT violation 
(p2m_change_type_one is called unconditionally without checking return 
value), I think it should be safe to do the current code here.
What's your comments?

Thanks,
-Kai

> > > +    d->arch.hvm_domain.vmx.status |= VMX_DOMAIN_PML_ENABLED;
> > I didn't see how this domain-wise flag is useful. Or if we really
> > want to go this way, you also need to clear this flag if vcpu pml
> > enable is failed in vcpu hotplug phase, since the flag itself means
> > all vcpus of the domain so we must keep this intention in all
> > places.
> IIUC we need this flag so that we know whether to enable PML on any
> new vcpus.  If we fail to enable PML on hotplug, we fail the hotplug
> (like for other vcpu bringup errors) so the invariant still holds.
>
> Cheers,
>
> Tim.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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