Altp2m use with PML can deadlock Xen

Altp2m use with PML can deadlock Xen

[Tamas K Lengyel]

Hi all,
I'm investigating an issue with altp2m that can easily be reproduced
and leads to a hypervisor deadlock when PML is available in hardware.
I haven't been able to trace down where the actual deadlock occurs.

The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
that calls p2m_change_type_one on all gfns that were recorded the PML
buffer. The problem occurs when the PML buffer full vmexit happens
while the active p2m is an altp2m. Switching  p2m_change_type_one to
work with the altp2m instead of the hostp2m however results in EPT
misconfiguration crashes.

Adding to the issue is that it seem to only occur when the altp2m has
remapped GFNs. Since PML records entries based on GFN leads me to
question whether it is safe at all to use PML when altp2m is used with
GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
remapped GFNs and my understanding is that it should be safe as long
as the GFNs being tracked by PML are never the remapped GFNs.

Booting Xen with ept=pml=0 resolves the issue.

If anyone has any insight into what might be happening, please let me know.

Thanks,
Tamas

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

[Xen-devel] Altp2m use with PML can deadlock Xen

[Tamas K Lengyel]

Hi all,
I'm investigating an issue with altp2m that can easily be reproduced
and leads to a hypervisor deadlock when PML is available in hardware.
I haven't been able to trace down where the actual deadlock occurs.

The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
that calls p2m_change_type_one on all gfns that were recorded the PML
buffer. The problem occurs when the PML buffer full vmexit happens
while the active p2m is an altp2m. Switching  p2m_change_type_one to
work with the altp2m instead of the hostp2m however results in EPT
misconfiguration crashes.

Adding to the issue is that it seem to only occur when the altp2m has
remapped GFNs. Since PML records entries based on GFN leads me to
question whether it is safe at all to use PML when altp2m is used with
GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
remapped GFNs and my understanding is that it should be safe as long
as the GFNs being tracked by PML are never the remapped GFNs.

Booting Xen with ept=pml=0 resolves the issue.

If anyone has any insight into what might be happening, please let me know.

Thanks,
Tamas

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

Re: Altp2m use with PML can deadlock Xen

[Andrew Cooper]

On 09/05/2019 14:38, Tamas K Lengyel wrote:
> Hi all,
> I'm investigating an issue with altp2m that can easily be reproduced
> and leads to a hypervisor deadlock when PML is available in hardware.
> I haven't been able to trace down where the actual deadlock occurs.
>
> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
> that calls p2m_change_type_one on all gfns that were recorded the PML
> buffer. The problem occurs when the PML buffer full vmexit happens
> while the active p2m is an altp2m. Switching  p2m_change_type_one to
> work with the altp2m instead of the hostp2m however results in EPT
> misconfiguration crashes.
>
> Adding to the issue is that it seem to only occur when the altp2m has
> remapped GFNs. Since PML records entries based on GFN leads me to
> question whether it is safe at all to use PML when altp2m is used with
> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
> remapped GFNs and my understanding is that it should be safe as long
> as the GFNs being tracked by PML are never the remapped GFNs.
>
> Booting Xen with ept=pml=0 resolves the issue.
>
> If anyone has any insight into what might be happening, please let me know.


I could have sworn that George spotted a problem here and fixed it.  I
shouldn't be surprised if we have more.

The problem that PML introduced (and this is mostly my fault, as I
suggested the buggy solution) is that the vmexit handler from one vcpu
pauses others to drain the PML queue into the dirty bitmap.  Overall I
wasn't happy with the design and I've got some ideas to improve it, but
within the scope of how altp2m was engineered, I proposed
domain_pause_except_self().

As it turns out, that is vulnerable to deadlocks when you get two vcpus
trying to pause each other and waiting for each other to become
de-scheduled.

I see this has been reused by the altp2m code, but it *should* be safe
to deadlocks now that it takes the hypercall_deadlock_mutext.

Anyway - sorry for not being more help, but I bet the problem is going
to be somewhere around vcpu pausing.

~Andrew

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

Re: [Xen-devel] Altp2m use with PML can deadlock Xen

[Andrew Cooper]

On 09/05/2019 14:38, Tamas K Lengyel wrote:
> Hi all,
> I'm investigating an issue with altp2m that can easily be reproduced
> and leads to a hypervisor deadlock when PML is available in hardware.
> I haven't been able to trace down where the actual deadlock occurs.
>
> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
> that calls p2m_change_type_one on all gfns that were recorded the PML
> buffer. The problem occurs when the PML buffer full vmexit happens
> while the active p2m is an altp2m. Switching  p2m_change_type_one to
> work with the altp2m instead of the hostp2m however results in EPT
> misconfiguration crashes.
>
> Adding to the issue is that it seem to only occur when the altp2m has
> remapped GFNs. Since PML records entries based on GFN leads me to
> question whether it is safe at all to use PML when altp2m is used with
> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
> remapped GFNs and my understanding is that it should be safe as long
> as the GFNs being tracked by PML are never the remapped GFNs.
>
> Booting Xen with ept=pml=0 resolves the issue.
>
> If anyone has any insight into what might be happening, please let me know.


I could have sworn that George spotted a problem here and fixed it.  I
shouldn't be surprised if we have more.

The problem that PML introduced (and this is mostly my fault, as I
suggested the buggy solution) is that the vmexit handler from one vcpu
pauses others to drain the PML queue into the dirty bitmap.  Overall I
wasn't happy with the design and I've got some ideas to improve it, but
within the scope of how altp2m was engineered, I proposed
domain_pause_except_self().

As it turns out, that is vulnerable to deadlocks when you get two vcpus
trying to pause each other and waiting for each other to become
de-scheduled.

I see this has been reused by the altp2m code, but it *should* be safe
to deadlocks now that it takes the hypercall_deadlock_mutext.

Anyway - sorry for not being more help, but I bet the problem is going
to be somewhere around vcpu pausing.

~Andrew

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

Re: Altp2m use with PML can deadlock Xen

[Tamas K Lengyel]

On Thu, May 9, 2019 at 10:19 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 09/05/2019 14:38, Tamas K Lengyel wrote:
> > Hi all,
> > I'm investigating an issue with altp2m that can easily be reproduced
> > and leads to a hypervisor deadlock when PML is available in hardware.
> > I haven't been able to trace down where the actual deadlock occurs.
> >
> > The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
> > that calls p2m_change_type_one on all gfns that were recorded the PML
> > buffer. The problem occurs when the PML buffer full vmexit happens
> > while the active p2m is an altp2m. Switching  p2m_change_type_one to
> > work with the altp2m instead of the hostp2m however results in EPT
> > misconfiguration crashes.
> >
> > Adding to the issue is that it seem to only occur when the altp2m has
> > remapped GFNs. Since PML records entries based on GFN leads me to
> > question whether it is safe at all to use PML when altp2m is used with
> > GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
> > remapped GFNs and my understanding is that it should be safe as long
> > as the GFNs being tracked by PML are never the remapped GFNs.
> >
> > Booting Xen with ept=pml=0 resolves the issue.
> >
> > If anyone has any insight into what might be happening, please let me know.
>
>
> I could have sworn that George spotted a problem here and fixed it.  I
> shouldn't be surprised if we have more.
>
> The problem that PML introduced (and this is mostly my fault, as I
> suggested the buggy solution) is that the vmexit handler from one vcpu
> pauses others to drain the PML queue into the dirty bitmap.  Overall I
> wasn't happy with the design and I've got some ideas to improve it, but
> within the scope of how altp2m was engineered, I proposed
> domain_pause_except_self().
>
> As it turns out, that is vulnerable to deadlocks when you get two vcpus
> trying to pause each other and waiting for each other to become
> de-scheduled.

Makes sense.

>
> I see this has been reused by the altp2m code, but it *should* be safe
> to deadlocks now that it takes the hypercall_deadlock_mutext.

Is that already in staging or your x86-next branch? I would like to
verify that the problem is still present or not with that change. I
tested with Xen 4.12 release and that definitely still deadlocks.

> Anyway - sorry for not being more help, but I bet the problem is going
> to be somewhere around vcpu pausing.

No problem, I appreciate the help.

Thanks,
Tamas

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

Re: [Xen-devel] Altp2m use with PML can deadlock Xen

[Tamas K Lengyel]

On Thu, May 9, 2019 at 10:19 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 09/05/2019 14:38, Tamas K Lengyel wrote:
> > Hi all,
> > I'm investigating an issue with altp2m that can easily be reproduced
> > and leads to a hypervisor deadlock when PML is available in hardware.
> > I haven't been able to trace down where the actual deadlock occurs.
> >
> > The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
> > that calls p2m_change_type_one on all gfns that were recorded the PML
> > buffer. The problem occurs when the PML buffer full vmexit happens
> > while the active p2m is an altp2m. Switching  p2m_change_type_one to
> > work with the altp2m instead of the hostp2m however results in EPT
> > misconfiguration crashes.
> >
> > Adding to the issue is that it seem to only occur when the altp2m has
> > remapped GFNs. Since PML records entries based on GFN leads me to
> > question whether it is safe at all to use PML when altp2m is used with
> > GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
> > remapped GFNs and my understanding is that it should be safe as long
> > as the GFNs being tracked by PML are never the remapped GFNs.
> >
> > Booting Xen with ept=pml=0 resolves the issue.
> >
> > If anyone has any insight into what might be happening, please let me know.
>
>
> I could have sworn that George spotted a problem here and fixed it.  I
> shouldn't be surprised if we have more.
>
> The problem that PML introduced (and this is mostly my fault, as I
> suggested the buggy solution) is that the vmexit handler from one vcpu
> pauses others to drain the PML queue into the dirty bitmap.  Overall I
> wasn't happy with the design and I've got some ideas to improve it, but
> within the scope of how altp2m was engineered, I proposed
> domain_pause_except_self().
>
> As it turns out, that is vulnerable to deadlocks when you get two vcpus
> trying to pause each other and waiting for each other to become
> de-scheduled.

Makes sense.

>
> I see this has been reused by the altp2m code, but it *should* be safe
> to deadlocks now that it takes the hypercall_deadlock_mutext.

Is that already in staging or your x86-next branch? I would like to
verify that the problem is still present or not with that change. I
tested with Xen 4.12 release and that definitely still deadlocks.

> Anyway - sorry for not being more help, but I bet the problem is going
> to be somewhere around vcpu pausing.

No problem, I appreciate the help.

Thanks,
Tamas

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

Re: Altp2m use with PML can deadlock Xen

[Razvan Cojocaru]

On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
>>> Hi all,
>>> I'm investigating an issue with altp2m that can easily be reproduced
>>> and leads to a hypervisor deadlock when PML is available in hardware.
>>> I haven't been able to trace down where the actual deadlock occurs.
>>>
>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
>>> that calls p2m_change_type_one on all gfns that were recorded the PML
>>> buffer. The problem occurs when the PML buffer full vmexit happens
>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
>>> work with the altp2m instead of the hostp2m however results in EPT
>>> misconfiguration crashes.
>>>
>>> Adding to the issue is that it seem to only occur when the altp2m has
>>> remapped GFNs. Since PML records entries based on GFN leads me to
>>> question whether it is safe at all to use PML when altp2m is used with
>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
>>> remapped GFNs and my understanding is that it should be safe as long
>>> as the GFNs being tracked by PML are never the remapped GFNs.
>>>
>>> Booting Xen with ept=pml=0 resolves the issue.
>>>
>>> If anyone has any insight into what might be happening, please let me know.
>>
>>
>> I could have sworn that George spotted a problem here and fixed it.  I
>> shouldn't be surprised if we have more.
>>
>> The problem that PML introduced (and this is mostly my fault, as I
>> suggested the buggy solution) is that the vmexit handler from one vcpu
>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
>> wasn't happy with the design and I've got some ideas to improve it, but
>> within the scope of how altp2m was engineered, I proposed
>> domain_pause_except_self().
>>
>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
>> trying to pause each other and waiting for each other to become
>> de-scheduled.
> 
> Makes sense.
> 
>>
>> I see this has been reused by the altp2m code, but it *should* be safe
>> to deadlocks now that it takes the hypercall_deadlock_mutext.
> 
> Is that already in staging or your x86-next branch? I would like to
> verify that the problem is still present or not with that change. I
> tested with Xen 4.12 release and that definitely still deadlocks.

I don't know if Andrew is talking about this patch (probably not, but it 
looks at least related):

http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363

Since there's a "Release-acked" tag on it, I think it's in 4.12.


Thanks,
Razvan

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

Re: [Xen-devel] Altp2m use with PML can deadlock Xen

[Razvan Cojocaru]

On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
>>> Hi all,
>>> I'm investigating an issue with altp2m that can easily be reproduced
>>> and leads to a hypervisor deadlock when PML is available in hardware.
>>> I haven't been able to trace down where the actual deadlock occurs.
>>>
>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
>>> that calls p2m_change_type_one on all gfns that were recorded the PML
>>> buffer. The problem occurs when the PML buffer full vmexit happens
>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
>>> work with the altp2m instead of the hostp2m however results in EPT
>>> misconfiguration crashes.
>>>
>>> Adding to the issue is that it seem to only occur when the altp2m has
>>> remapped GFNs. Since PML records entries based on GFN leads me to
>>> question whether it is safe at all to use PML when altp2m is used with
>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
>>> remapped GFNs and my understanding is that it should be safe as long
>>> as the GFNs being tracked by PML are never the remapped GFNs.
>>>
>>> Booting Xen with ept=pml=0 resolves the issue.
>>>
>>> If anyone has any insight into what might be happening, please let me know.
>>
>>
>> I could have sworn that George spotted a problem here and fixed it.  I
>> shouldn't be surprised if we have more.
>>
>> The problem that PML introduced (and this is mostly my fault, as I
>> suggested the buggy solution) is that the vmexit handler from one vcpu
>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
>> wasn't happy with the design and I've got some ideas to improve it, but
>> within the scope of how altp2m was engineered, I proposed
>> domain_pause_except_self().
>>
>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
>> trying to pause each other and waiting for each other to become
>> de-scheduled.
> 
> Makes sense.
> 
>>
>> I see this has been reused by the altp2m code, but it *should* be safe
>> to deadlocks now that it takes the hypercall_deadlock_mutext.
> 
> Is that already in staging or your x86-next branch? I would like to
> verify that the problem is still present or not with that change. I
> tested with Xen 4.12 release and that definitely still deadlocks.

I don't know if Andrew is talking about this patch (probably not, but it 
looks at least related):

http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363

Since there's a "Release-acked" tag on it, I think it's in 4.12.


Thanks,
Razvan

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

Re: Altp2m use with PML can deadlock Xen

[Andrew Cooper]

On 10/05/2019 15:53, Razvan Cojocaru wrote:
> On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
>> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
>> <andrew.cooper3@citrix.com> wrote:
>>>
>>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
>>>> Hi all,
>>>> I'm investigating an issue with altp2m that can easily be reproduced
>>>> and leads to a hypervisor deadlock when PML is available in hardware.
>>>> I haven't been able to trace down where the actual deadlock occurs.
>>>>
>>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
>>>> that calls p2m_change_type_one on all gfns that were recorded the PML
>>>> buffer. The problem occurs when the PML buffer full vmexit happens
>>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
>>>> work with the altp2m instead of the hostp2m however results in EPT
>>>> misconfiguration crashes.
>>>>
>>>> Adding to the issue is that it seem to only occur when the altp2m has
>>>> remapped GFNs. Since PML records entries based on GFN leads me to
>>>> question whether it is safe at all to use PML when altp2m is used with
>>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
>>>> remapped GFNs and my understanding is that it should be safe as long
>>>> as the GFNs being tracked by PML are never the remapped GFNs.
>>>>
>>>> Booting Xen with ept=pml=0 resolves the issue.
>>>>
>>>> If anyone has any insight into what might be happening, please let
>>>> me know.
>>>
>>>
>>> I could have sworn that George spotted a problem here and fixed it.  I
>>> shouldn't be surprised if we have more.
>>>
>>> The problem that PML introduced (and this is mostly my fault, as I
>>> suggested the buggy solution) is that the vmexit handler from one vcpu
>>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
>>> wasn't happy with the design and I've got some ideas to improve it, but
>>> within the scope of how altp2m was engineered, I proposed
>>> domain_pause_except_self().
>>>
>>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
>>> trying to pause each other and waiting for each other to become
>>> de-scheduled.
>>
>> Makes sense.
>>
>>>
>>> I see this has been reused by the altp2m code, but it *should* be safe
>>> to deadlocks now that it takes the hypercall_deadlock_mutext.
>>
>> Is that already in staging or your x86-next branch? I would like to
>> verify that the problem is still present or not with that change. I
>> tested with Xen 4.12 release and that definitely still deadlocks.
>
> I don't know if Andrew is talking about this patch (probably not, but
> it looks at least related):
>
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
>

I was referring to 29d28b2919 which is also in 4.12 as it turns out. 
That said, 24d5282527 might in practice be the cause of the deadlock, so
I'd first experiment with taking that fix out.

I know for certain that it won't be tested with PML enabled, because the
use of PML is incompatible with write-protecting guest pagetables.

~Andrew

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

Re: [Xen-devel] Altp2m use with PML can deadlock Xen

[Andrew Cooper]

On 10/05/2019 15:53, Razvan Cojocaru wrote:
> On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
>> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
>> <andrew.cooper3@citrix.com> wrote:
>>>
>>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
>>>> Hi all,
>>>> I'm investigating an issue with altp2m that can easily be reproduced
>>>> and leads to a hypervisor deadlock when PML is available in hardware.
>>>> I haven't been able to trace down where the actual deadlock occurs.
>>>>
>>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
>>>> that calls p2m_change_type_one on all gfns that were recorded the PML
>>>> buffer. The problem occurs when the PML buffer full vmexit happens
>>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
>>>> work with the altp2m instead of the hostp2m however results in EPT
>>>> misconfiguration crashes.
>>>>
>>>> Adding to the issue is that it seem to only occur when the altp2m has
>>>> remapped GFNs. Since PML records entries based on GFN leads me to
>>>> question whether it is safe at all to use PML when altp2m is used with
>>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
>>>> remapped GFNs and my understanding is that it should be safe as long
>>>> as the GFNs being tracked by PML are never the remapped GFNs.
>>>>
>>>> Booting Xen with ept=pml=0 resolves the issue.
>>>>
>>>> If anyone has any insight into what might be happening, please let
>>>> me know.
>>>
>>>
>>> I could have sworn that George spotted a problem here and fixed it.  I
>>> shouldn't be surprised if we have more.
>>>
>>> The problem that PML introduced (and this is mostly my fault, as I
>>> suggested the buggy solution) is that the vmexit handler from one vcpu
>>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
>>> wasn't happy with the design and I've got some ideas to improve it, but
>>> within the scope of how altp2m was engineered, I proposed
>>> domain_pause_except_self().
>>>
>>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
>>> trying to pause each other and waiting for each other to become
>>> de-scheduled.
>>
>> Makes sense.
>>
>>>
>>> I see this has been reused by the altp2m code, but it *should* be safe
>>> to deadlocks now that it takes the hypercall_deadlock_mutext.
>>
>> Is that already in staging or your x86-next branch? I would like to
>> verify that the problem is still present or not with that change. I
>> tested with Xen 4.12 release and that definitely still deadlocks.
>
> I don't know if Andrew is talking about this patch (probably not, but
> it looks at least related):
>
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
>

I was referring to 29d28b2919 which is also in 4.12 as it turns out. 
That said, 24d5282527 might in practice be the cause of the deadlock, so
I'd first experiment with taking that fix out.

I know for certain that it won't be tested with PML enabled, because the
use of PML is incompatible with write-protecting guest pagetables.

~Andrew

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

Re: Altp2m use with PML can deadlock Xen

[Tamas K Lengyel]

On Fri, May 10, 2019 at 8:59 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 10/05/2019 15:53, Razvan Cojocaru wrote:
> > On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
> >> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
> >> <andrew.cooper3@citrix.com> wrote:
> >>>
> >>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
> >>>> Hi all,
> >>>> I'm investigating an issue with altp2m that can easily be reproduced
> >>>> and leads to a hypervisor deadlock when PML is available in hardware.
> >>>> I haven't been able to trace down where the actual deadlock occurs.
> >>>>
> >>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
> >>>> that calls p2m_change_type_one on all gfns that were recorded the PML
> >>>> buffer. The problem occurs when the PML buffer full vmexit happens
> >>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
> >>>> work with the altp2m instead of the hostp2m however results in EPT
> >>>> misconfiguration crashes.
> >>>>
> >>>> Adding to the issue is that it seem to only occur when the altp2m has
> >>>> remapped GFNs. Since PML records entries based on GFN leads me to
> >>>> question whether it is safe at all to use PML when altp2m is used with
> >>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
> >>>> remapped GFNs and my understanding is that it should be safe as long
> >>>> as the GFNs being tracked by PML are never the remapped GFNs.
> >>>>
> >>>> Booting Xen with ept=pml=0 resolves the issue.
> >>>>
> >>>> If anyone has any insight into what might be happening, please let
> >>>> me know.
> >>>
> >>>
> >>> I could have sworn that George spotted a problem here and fixed it.  I
> >>> shouldn't be surprised if we have more.
> >>>
> >>> The problem that PML introduced (and this is mostly my fault, as I
> >>> suggested the buggy solution) is that the vmexit handler from one vcpu
> >>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
> >>> wasn't happy with the design and I've got some ideas to improve it, but
> >>> within the scope of how altp2m was engineered, I proposed
> >>> domain_pause_except_self().
> >>>
> >>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
> >>> trying to pause each other and waiting for each other to become
> >>> de-scheduled.
> >>
> >> Makes sense.
> >>
> >>>
> >>> I see this has been reused by the altp2m code, but it *should* be safe
> >>> to deadlocks now that it takes the hypercall_deadlock_mutext.
> >>
> >> Is that already in staging or your x86-next branch? I would like to
> >> verify that the problem is still present or not with that change. I
> >> tested with Xen 4.12 release and that definitely still deadlocks.
> >
> > I don't know if Andrew is talking about this patch (probably not, but
> > it looks at least related):
> >
> > http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
> >
>
> I was referring to 29d28b2919 which is also in 4.12 as it turns out.
> That said, 24d5282527 might in practice be the cause of the deadlock, so
> I'd first experiment with taking that fix out.
>
> I know for certain that it won't be tested with PML enabled, because the
> use of PML is incompatible with write-protecting guest pagetables.
>

Sounds like it's the safe bet to just have PML be disabled for when
introspection is used. I would say it would be even better if the use
of PML could be controlled on a per-guest base instead of the current
global on/off switch. That way it could be disabled only for the
introspected domains.

I'll do some more experimentation when I get some free time but two
observations that speak against the vCPUs trying to pause each other
being the culprit is that:
- the deadlock doesn't happen with xen-access' altp2m use, it only
happens when there are remapped gfn's in the altp2m views
- I've added a domain_pause/unpause to the PML flusher before it
enters the flush loop but I still got a deadlock

Tamas

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

Re: [Xen-devel] Altp2m use with PML can deadlock Xen

[Tamas K Lengyel]

On Fri, May 10, 2019 at 8:59 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 10/05/2019 15:53, Razvan Cojocaru wrote:
> > On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
> >> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
> >> <andrew.cooper3@citrix.com> wrote:
> >>>
> >>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
> >>>> Hi all,
> >>>> I'm investigating an issue with altp2m that can easily be reproduced
> >>>> and leads to a hypervisor deadlock when PML is available in hardware.
> >>>> I haven't been able to trace down where the actual deadlock occurs.
> >>>>
> >>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
> >>>> that calls p2m_change_type_one on all gfns that were recorded the PML
> >>>> buffer. The problem occurs when the PML buffer full vmexit happens
> >>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
> >>>> work with the altp2m instead of the hostp2m however results in EPT
> >>>> misconfiguration crashes.
> >>>>
> >>>> Adding to the issue is that it seem to only occur when the altp2m has
> >>>> remapped GFNs. Since PML records entries based on GFN leads me to
> >>>> question whether it is safe at all to use PML when altp2m is used with
> >>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
> >>>> remapped GFNs and my understanding is that it should be safe as long
> >>>> as the GFNs being tracked by PML are never the remapped GFNs.
> >>>>
> >>>> Booting Xen with ept=pml=0 resolves the issue.
> >>>>
> >>>> If anyone has any insight into what might be happening, please let
> >>>> me know.
> >>>
> >>>
> >>> I could have sworn that George spotted a problem here and fixed it.  I
> >>> shouldn't be surprised if we have more.
> >>>
> >>> The problem that PML introduced (and this is mostly my fault, as I
> >>> suggested the buggy solution) is that the vmexit handler from one vcpu
> >>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
> >>> wasn't happy with the design and I've got some ideas to improve it, but
> >>> within the scope of how altp2m was engineered, I proposed
> >>> domain_pause_except_self().
> >>>
> >>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
> >>> trying to pause each other and waiting for each other to become
> >>> de-scheduled.
> >>
> >> Makes sense.
> >>
> >>>
> >>> I see this has been reused by the altp2m code, but it *should* be safe
> >>> to deadlocks now that it takes the hypercall_deadlock_mutext.
> >>
> >> Is that already in staging or your x86-next branch? I would like to
> >> verify that the problem is still present or not with that change. I
> >> tested with Xen 4.12 release and that definitely still deadlocks.
> >
> > I don't know if Andrew is talking about this patch (probably not, but
> > it looks at least related):
> >
> > http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
> >
>
> I was referring to 29d28b2919 which is also in 4.12 as it turns out.
> That said, 24d5282527 might in practice be the cause of the deadlock, so
> I'd first experiment with taking that fix out.
>
> I know for certain that it won't be tested with PML enabled, because the
> use of PML is incompatible with write-protecting guest pagetables.
>

Sounds like it's the safe bet to just have PML be disabled for when
introspection is used. I would say it would be even better if the use
of PML could be controlled on a per-guest base instead of the current
global on/off switch. That way it could be disabled only for the
introspected domains.

I'll do some more experimentation when I get some free time but two
observations that speak against the vCPUs trying to pause each other
being the culprit is that:
- the deadlock doesn't happen with xen-access' altp2m use, it only
happens when there are remapped gfn's in the altp2m views
- I've added a domain_pause/unpause to the PML flusher before it
enters the flush loop but I still got a deadlock

Tamas

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

Re: Altp2m use with PML can deadlock Xen

[Andrew Cooper]

On 10/05/2019 16:09, Tamas K Lengyel wrote:
> On Fri, May 10, 2019 at 8:59 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 10/05/2019 15:53, Razvan Cojocaru wrote:
>>> On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
>>>> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
>>>> <andrew.cooper3@citrix.com> wrote:
>>>>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
>>>>>> Hi all,
>>>>>> I'm investigating an issue with altp2m that can easily be reproduced
>>>>>> and leads to a hypervisor deadlock when PML is available in hardware.
>>>>>> I haven't been able to trace down where the actual deadlock occurs.
>>>>>>
>>>>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
>>>>>> that calls p2m_change_type_one on all gfns that were recorded the PML
>>>>>> buffer. The problem occurs when the PML buffer full vmexit happens
>>>>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
>>>>>> work with the altp2m instead of the hostp2m however results in EPT
>>>>>> misconfiguration crashes.
>>>>>>
>>>>>> Adding to the issue is that it seem to only occur when the altp2m has
>>>>>> remapped GFNs. Since PML records entries based on GFN leads me to
>>>>>> question whether it is safe at all to use PML when altp2m is used with
>>>>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
>>>>>> remapped GFNs and my understanding is that it should be safe as long
>>>>>> as the GFNs being tracked by PML are never the remapped GFNs.
>>>>>>
>>>>>> Booting Xen with ept=pml=0 resolves the issue.
>>>>>>
>>>>>> If anyone has any insight into what might be happening, please let
>>>>>> me know.
>>>>>
>>>>> I could have sworn that George spotted a problem here and fixed it.  I
>>>>> shouldn't be surprised if we have more.
>>>>>
>>>>> The problem that PML introduced (and this is mostly my fault, as I
>>>>> suggested the buggy solution) is that the vmexit handler from one vcpu
>>>>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
>>>>> wasn't happy with the design and I've got some ideas to improve it, but
>>>>> within the scope of how altp2m was engineered, I proposed
>>>>> domain_pause_except_self().
>>>>>
>>>>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
>>>>> trying to pause each other and waiting for each other to become
>>>>> de-scheduled.
>>>> Makes sense.
>>>>
>>>>> I see this has been reused by the altp2m code, but it *should* be safe
>>>>> to deadlocks now that it takes the hypercall_deadlock_mutext.
>>>> Is that already in staging or your x86-next branch? I would like to
>>>> verify that the problem is still present or not with that change. I
>>>> tested with Xen 4.12 release and that definitely still deadlocks.
>>> I don't know if Andrew is talking about this patch (probably not, but
>>> it looks at least related):
>>>
>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
>>>
>> I was referring to 29d28b2919 which is also in 4.12 as it turns out.
>> That said, 24d5282527 might in practice be the cause of the deadlock, so
>> I'd first experiment with taking that fix out.
>>
>> I know for certain that it won't be tested with PML enabled, because the
>> use of PML is incompatible with write-protecting guest pagetables.
>>
> Sounds like it's the safe bet to just have PML be disabled for when
> introspection is used. I would say it would be even better if the use
> of PML could be controlled on a per-guest base instead of the current
> global on/off switch. That way it could be disabled only for the
> introspected domains.
>
> I'll do some more experimentation when I get some free time but two
> observations that speak against the vCPUs trying to pause each other
> being the culprit is that:
> - the deadlock doesn't happen with xen-access' altp2m use, it only
> happens when there are remapped gfn's in the altp2m views
> - I've added a domain_pause/unpause to the PML flusher before it
> enters the flush loop but I still got a deadlock

Do you have a minimal repro of the deadlock you could share?

Even if it is a combo PML+altp2m problem, we should fix the issue,
because there are VMI usecases which don't care about write-protecting
guest pagetables, and we don't want to prevent those cases from using PML.

Thanks,

~Andrew

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

Re: [Xen-devel] Altp2m use with PML can deadlock Xen

[Andrew Cooper]

On 10/05/2019 16:09, Tamas K Lengyel wrote:
> On Fri, May 10, 2019 at 8:59 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 10/05/2019 15:53, Razvan Cojocaru wrote:
>>> On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
>>>> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
>>>> <andrew.cooper3@citrix.com> wrote:
>>>>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
>>>>>> Hi all,
>>>>>> I'm investigating an issue with altp2m that can easily be reproduced
>>>>>> and leads to a hypervisor deadlock when PML is available in hardware.
>>>>>> I haven't been able to trace down where the actual deadlock occurs.
>>>>>>
>>>>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
>>>>>> that calls p2m_change_type_one on all gfns that were recorded the PML
>>>>>> buffer. The problem occurs when the PML buffer full vmexit happens
>>>>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
>>>>>> work with the altp2m instead of the hostp2m however results in EPT
>>>>>> misconfiguration crashes.
>>>>>>
>>>>>> Adding to the issue is that it seem to only occur when the altp2m has
>>>>>> remapped GFNs. Since PML records entries based on GFN leads me to
>>>>>> question whether it is safe at all to use PML when altp2m is used with
>>>>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
>>>>>> remapped GFNs and my understanding is that it should be safe as long
>>>>>> as the GFNs being tracked by PML are never the remapped GFNs.
>>>>>>
>>>>>> Booting Xen with ept=pml=0 resolves the issue.
>>>>>>
>>>>>> If anyone has any insight into what might be happening, please let
>>>>>> me know.
>>>>>
>>>>> I could have sworn that George spotted a problem here and fixed it.  I
>>>>> shouldn't be surprised if we have more.
>>>>>
>>>>> The problem that PML introduced (and this is mostly my fault, as I
>>>>> suggested the buggy solution) is that the vmexit handler from one vcpu
>>>>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
>>>>> wasn't happy with the design and I've got some ideas to improve it, but
>>>>> within the scope of how altp2m was engineered, I proposed
>>>>> domain_pause_except_self().
>>>>>
>>>>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
>>>>> trying to pause each other and waiting for each other to become
>>>>> de-scheduled.
>>>> Makes sense.
>>>>
>>>>> I see this has been reused by the altp2m code, but it *should* be safe
>>>>> to deadlocks now that it takes the hypercall_deadlock_mutext.
>>>> Is that already in staging or your x86-next branch? I would like to
>>>> verify that the problem is still present or not with that change. I
>>>> tested with Xen 4.12 release and that definitely still deadlocks.
>>> I don't know if Andrew is talking about this patch (probably not, but
>>> it looks at least related):
>>>
>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
>>>
>> I was referring to 29d28b2919 which is also in 4.12 as it turns out.
>> That said, 24d5282527 might in practice be the cause of the deadlock, so
>> I'd first experiment with taking that fix out.
>>
>> I know for certain that it won't be tested with PML enabled, because the
>> use of PML is incompatible with write-protecting guest pagetables.
>>
> Sounds like it's the safe bet to just have PML be disabled for when
> introspection is used. I would say it would be even better if the use
> of PML could be controlled on a per-guest base instead of the current
> global on/off switch. That way it could be disabled only for the
> introspected domains.
>
> I'll do some more experimentation when I get some free time but two
> observations that speak against the vCPUs trying to pause each other
> being the culprit is that:
> - the deadlock doesn't happen with xen-access' altp2m use, it only
> happens when there are remapped gfn's in the altp2m views
> - I've added a domain_pause/unpause to the PML flusher before it
> enters the flush loop but I still got a deadlock

Do you have a minimal repro of the deadlock you could share?

Even if it is a combo PML+altp2m problem, we should fix the issue,
because there are VMI usecases which don't care about write-protecting
guest pagetables, and we don't want to prevent those cases from using PML.

Thanks,

~Andrew

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

Re: Altp2m use with PML can deadlock Xen

[Tamas K Lengyel]

On Fri, May 10, 2019 at 9:21 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 10/05/2019 16:09, Tamas K Lengyel wrote:
> > On Fri, May 10, 2019 at 8:59 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> On 10/05/2019 15:53, Razvan Cojocaru wrote:
> >>> On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
> >>>> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
> >>>> <andrew.cooper3@citrix.com> wrote:
> >>>>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
> >>>>>> Hi all,
> >>>>>> I'm investigating an issue with altp2m that can easily be reproduced
> >>>>>> and leads to a hypervisor deadlock when PML is available in hardware.
> >>>>>> I haven't been able to trace down where the actual deadlock occurs.
> >>>>>>
> >>>>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
> >>>>>> that calls p2m_change_type_one on all gfns that were recorded the PML
> >>>>>> buffer. The problem occurs when the PML buffer full vmexit happens
> >>>>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
> >>>>>> work with the altp2m instead of the hostp2m however results in EPT
> >>>>>> misconfiguration crashes.
> >>>>>>
> >>>>>> Adding to the issue is that it seem to only occur when the altp2m has
> >>>>>> remapped GFNs. Since PML records entries based on GFN leads me to
> >>>>>> question whether it is safe at all to use PML when altp2m is used with
> >>>>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
> >>>>>> remapped GFNs and my understanding is that it should be safe as long
> >>>>>> as the GFNs being tracked by PML are never the remapped GFNs.
> >>>>>>
> >>>>>> Booting Xen with ept=pml=0 resolves the issue.
> >>>>>>
> >>>>>> If anyone has any insight into what might be happening, please let
> >>>>>> me know.
> >>>>>
> >>>>> I could have sworn that George spotted a problem here and fixed it.  I
> >>>>> shouldn't be surprised if we have more.
> >>>>>
> >>>>> The problem that PML introduced (and this is mostly my fault, as I
> >>>>> suggested the buggy solution) is that the vmexit handler from one vcpu
> >>>>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
> >>>>> wasn't happy with the design and I've got some ideas to improve it, but
> >>>>> within the scope of how altp2m was engineered, I proposed
> >>>>> domain_pause_except_self().
> >>>>>
> >>>>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
> >>>>> trying to pause each other and waiting for each other to become
> >>>>> de-scheduled.
> >>>> Makes sense.
> >>>>
> >>>>> I see this has been reused by the altp2m code, but it *should* be safe
> >>>>> to deadlocks now that it takes the hypercall_deadlock_mutext.
> >>>> Is that already in staging or your x86-next branch? I would like to
> >>>> verify that the problem is still present or not with that change. I
> >>>> tested with Xen 4.12 release and that definitely still deadlocks.
> >>> I don't know if Andrew is talking about this patch (probably not, but
> >>> it looks at least related):
> >>>
> >>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
> >>>
> >> I was referring to 29d28b2919 which is also in 4.12 as it turns out.
> >> That said, 24d5282527 might in practice be the cause of the deadlock, so
> >> I'd first experiment with taking that fix out.
> >>
> >> I know for certain that it won't be tested with PML enabled, because the
> >> use of PML is incompatible with write-protecting guest pagetables.
> >>
> > Sounds like it's the safe bet to just have PML be disabled for when
> > introspection is used. I would say it would be even better if the use
> > of PML could be controlled on a per-guest base instead of the current
> > global on/off switch. That way it could be disabled only for the
> > introspected domains.
> >
> > I'll do some more experimentation when I get some free time but two
> > observations that speak against the vCPUs trying to pause each other
> > being the culprit is that:
> > - the deadlock doesn't happen with xen-access' altp2m use, it only
> > happens when there are remapped gfn's in the altp2m views
> > - I've added a domain_pause/unpause to the PML flusher before it
> > enters the flush loop but I still got a deadlock
>
> Do you have a minimal repro of the deadlock you could share?

The deadlock is easily reproducible but right now it's only with a
full setup of DRAKVUF (just the standard setup as described on
drakvuf.com). Also, the deadlock only triggers after connecting to the
VM being monitored over VNC (that's what triggers the VGA logdirty
pages filling up the PML buffer).

> Even if it is a combo PML+altp2m problem, we should fix the issue,
> because there are VMI usecases which don't care about write-protecting
> guest pagetables, and we don't want to prevent those cases from using PML.

Agree. For my use-case it's not an issue - I already just posted the
updated instructions to boot with ept=pml=0. But it's better to fix it
before it leads to other problems down the road.

Tamas

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

Re: [Xen-devel] Altp2m use with PML can deadlock Xen

[Tamas K Lengyel]

On Fri, May 10, 2019 at 9:21 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 10/05/2019 16:09, Tamas K Lengyel wrote:
> > On Fri, May 10, 2019 at 8:59 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> On 10/05/2019 15:53, Razvan Cojocaru wrote:
> >>> On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
> >>>> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
> >>>> <andrew.cooper3@citrix.com> wrote:
> >>>>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
> >>>>>> Hi all,
> >>>>>> I'm investigating an issue with altp2m that can easily be reproduced
> >>>>>> and leads to a hypervisor deadlock when PML is available in hardware.
> >>>>>> I haven't been able to trace down where the actual deadlock occurs.
> >>>>>>
> >>>>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
> >>>>>> that calls p2m_change_type_one on all gfns that were recorded the PML
> >>>>>> buffer. The problem occurs when the PML buffer full vmexit happens
> >>>>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
> >>>>>> work with the altp2m instead of the hostp2m however results in EPT
> >>>>>> misconfiguration crashes.
> >>>>>>
> >>>>>> Adding to the issue is that it seem to only occur when the altp2m has
> >>>>>> remapped GFNs. Since PML records entries based on GFN leads me to
> >>>>>> question whether it is safe at all to use PML when altp2m is used with
> >>>>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
> >>>>>> remapped GFNs and my understanding is that it should be safe as long
> >>>>>> as the GFNs being tracked by PML are never the remapped GFNs.
> >>>>>>
> >>>>>> Booting Xen with ept=pml=0 resolves the issue.
> >>>>>>
> >>>>>> If anyone has any insight into what might be happening, please let
> >>>>>> me know.
> >>>>>
> >>>>> I could have sworn that George spotted a problem here and fixed it.  I
> >>>>> shouldn't be surprised if we have more.
> >>>>>
> >>>>> The problem that PML introduced (and this is mostly my fault, as I
> >>>>> suggested the buggy solution) is that the vmexit handler from one vcpu
> >>>>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
> >>>>> wasn't happy with the design and I've got some ideas to improve it, but
> >>>>> within the scope of how altp2m was engineered, I proposed
> >>>>> domain_pause_except_self().
> >>>>>
> >>>>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
> >>>>> trying to pause each other and waiting for each other to become
> >>>>> de-scheduled.
> >>>> Makes sense.
> >>>>
> >>>>> I see this has been reused by the altp2m code, but it *should* be safe
> >>>>> to deadlocks now that it takes the hypercall_deadlock_mutext.
> >>>> Is that already in staging or your x86-next branch? I would like to
> >>>> verify that the problem is still present or not with that change. I
> >>>> tested with Xen 4.12 release and that definitely still deadlocks.
> >>> I don't know if Andrew is talking about this patch (probably not, but
> >>> it looks at least related):
> >>>
> >>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
> >>>
> >> I was referring to 29d28b2919 which is also in 4.12 as it turns out.
> >> That said, 24d5282527 might in practice be the cause of the deadlock, so
> >> I'd first experiment with taking that fix out.
> >>
> >> I know for certain that it won't be tested with PML enabled, because the
> >> use of PML is incompatible with write-protecting guest pagetables.
> >>
> > Sounds like it's the safe bet to just have PML be disabled for when
> > introspection is used. I would say it would be even better if the use
> > of PML could be controlled on a per-guest base instead of the current
> > global on/off switch. That way it could be disabled only for the
> > introspected domains.
> >
> > I'll do some more experimentation when I get some free time but two
> > observations that speak against the vCPUs trying to pause each other
> > being the culprit is that:
> > - the deadlock doesn't happen with xen-access' altp2m use, it only
> > happens when there are remapped gfn's in the altp2m views
> > - I've added a domain_pause/unpause to the PML flusher before it
> > enters the flush loop but I still got a deadlock
>
> Do you have a minimal repro of the deadlock you could share?

The deadlock is easily reproducible but right now it's only with a
full setup of DRAKVUF (just the standard setup as described on
drakvuf.com). Also, the deadlock only triggers after connecting to the
VM being monitored over VNC (that's what triggers the VGA logdirty
pages filling up the PML buffer).

> Even if it is a combo PML+altp2m problem, we should fix the issue,
> because there are VMI usecases which don't care about write-protecting
> guest pagetables, and we don't want to prevent those cases from using PML.

Agree. For my use-case it's not an issue - I already just posted the
updated instructions to boot with ept=pml=0. But it's better to fix it
before it leads to other problems down the road.

Tamas

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

Re: Altp2m use with PML can deadlock Xen

[Andrew Cooper]

On 10/05/2019 16:29, Tamas K Lengyel wrote:
> On Fri, May 10, 2019 at 9:21 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 10/05/2019 16:09, Tamas K Lengyel wrote:
>>> On Fri, May 10, 2019 at 8:59 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>>> On 10/05/2019 15:53, Razvan Cojocaru wrote:
>>>>> On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
>>>>>> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
>>>>>> <andrew.cooper3@citrix.com> wrote:
>>>>>>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
>>>>>>>> Hi all,
>>>>>>>> I'm investigating an issue with altp2m that can easily be reproduced
>>>>>>>> and leads to a hypervisor deadlock when PML is available in hardware.
>>>>>>>> I haven't been able to trace down where the actual deadlock occurs.
>>>>>>>>
>>>>>>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
>>>>>>>> that calls p2m_change_type_one on all gfns that were recorded the PML
>>>>>>>> buffer. The problem occurs when the PML buffer full vmexit happens
>>>>>>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
>>>>>>>> work with the altp2m instead of the hostp2m however results in EPT
>>>>>>>> misconfiguration crashes.
>>>>>>>>
>>>>>>>> Adding to the issue is that it seem to only occur when the altp2m has
>>>>>>>> remapped GFNs. Since PML records entries based on GFN leads me to
>>>>>>>> question whether it is safe at all to use PML when altp2m is used with
>>>>>>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
>>>>>>>> remapped GFNs and my understanding is that it should be safe as long
>>>>>>>> as the GFNs being tracked by PML are never the remapped GFNs.
>>>>>>>>
>>>>>>>> Booting Xen with ept=pml=0 resolves the issue.
>>>>>>>>
>>>>>>>> If anyone has any insight into what might be happening, please let
>>>>>>>> me know.
>>>>>>> I could have sworn that George spotted a problem here and fixed it.  I
>>>>>>> shouldn't be surprised if we have more.
>>>>>>>
>>>>>>> The problem that PML introduced (and this is mostly my fault, as I
>>>>>>> suggested the buggy solution) is that the vmexit handler from one vcpu
>>>>>>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
>>>>>>> wasn't happy with the design and I've got some ideas to improve it, but
>>>>>>> within the scope of how altp2m was engineered, I proposed
>>>>>>> domain_pause_except_self().
>>>>>>>
>>>>>>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
>>>>>>> trying to pause each other and waiting for each other to become
>>>>>>> de-scheduled.
>>>>>> Makes sense.
>>>>>>
>>>>>>> I see this has been reused by the altp2m code, but it *should* be safe
>>>>>>> to deadlocks now that it takes the hypercall_deadlock_mutext.
>>>>>> Is that already in staging or your x86-next branch? I would like to
>>>>>> verify that the problem is still present or not with that change. I
>>>>>> tested with Xen 4.12 release and that definitely still deadlocks.
>>>>> I don't know if Andrew is talking about this patch (probably not, but
>>>>> it looks at least related):
>>>>>
>>>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
>>>>>
>>>> I was referring to 29d28b2919 which is also in 4.12 as it turns out.
>>>> That said, 24d5282527 might in practice be the cause of the deadlock, so
>>>> I'd first experiment with taking that fix out.
>>>>
>>>> I know for certain that it won't be tested with PML enabled, because the
>>>> use of PML is incompatible with write-protecting guest pagetables.
>>>>
>>> Sounds like it's the safe bet to just have PML be disabled for when
>>> introspection is used. I would say it would be even better if the use
>>> of PML could be controlled on a per-guest base instead of the current
>>> global on/off switch. That way it could be disabled only for the
>>> introspected domains.
>>>
>>> I'll do some more experimentation when I get some free time but two
>>> observations that speak against the vCPUs trying to pause each other
>>> being the culprit is that:
>>> - the deadlock doesn't happen with xen-access' altp2m use, it only
>>> happens when there are remapped gfn's in the altp2m views
>>> - I've added a domain_pause/unpause to the PML flusher before it
>>> enters the flush loop but I still got a deadlock
>> Do you have a minimal repro of the deadlock you could share?
> The deadlock is easily reproducible but right now it's only with a
> full setup of DRAKVUF (just the standard setup as described on
> drakvuf.com). Also, the deadlock only triggers after connecting to the
> VM being monitored over VNC (that's what triggers the VGA logdirty
> pages filling up the PML buffer).

Ok - that's probably easy enough to investigate from, when I (or someone
else) can find some copious free time.

~Andrew

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

Re: [Xen-devel] Altp2m use with PML can deadlock Xen

[Andrew Cooper]

On 10/05/2019 16:29, Tamas K Lengyel wrote:
> On Fri, May 10, 2019 at 9:21 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 10/05/2019 16:09, Tamas K Lengyel wrote:
>>> On Fri, May 10, 2019 at 8:59 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>>> On 10/05/2019 15:53, Razvan Cojocaru wrote:
>>>>> On 5/10/19 5:42 PM, Tamas K Lengyel wrote:
>>>>>> On Thu, May 9, 2019 at 10:19 AM Andrew Cooper
>>>>>> <andrew.cooper3@citrix.com> wrote:
>>>>>>> On 09/05/2019 14:38, Tamas K Lengyel wrote:
>>>>>>>> Hi all,
>>>>>>>> I'm investigating an issue with altp2m that can easily be reproduced
>>>>>>>> and leads to a hypervisor deadlock when PML is available in hardware.
>>>>>>>> I haven't been able to trace down where the actual deadlock occurs.
>>>>>>>>
>>>>>>>> The problem seem to stem from hvm/vmx/vmcs.c:vmx_vcpu_flush_pml_buffer
>>>>>>>> that calls p2m_change_type_one on all gfns that were recorded the PML
>>>>>>>> buffer. The problem occurs when the PML buffer full vmexit happens
>>>>>>>> while the active p2m is an altp2m. Switching  p2m_change_type_one to
>>>>>>>> work with the altp2m instead of the hostp2m however results in EPT
>>>>>>>> misconfiguration crashes.
>>>>>>>>
>>>>>>>> Adding to the issue is that it seem to only occur when the altp2m has
>>>>>>>> remapped GFNs. Since PML records entries based on GFN leads me to
>>>>>>>> question whether it is safe at all to use PML when altp2m is used with
>>>>>>>> GFN remapping. However, AFAICT the GFNs in the PML buffer are not the
>>>>>>>> remapped GFNs and my understanding is that it should be safe as long
>>>>>>>> as the GFNs being tracked by PML are never the remapped GFNs.
>>>>>>>>
>>>>>>>> Booting Xen with ept=pml=0 resolves the issue.
>>>>>>>>
>>>>>>>> If anyone has any insight into what might be happening, please let
>>>>>>>> me know.
>>>>>>> I could have sworn that George spotted a problem here and fixed it.  I
>>>>>>> shouldn't be surprised if we have more.
>>>>>>>
>>>>>>> The problem that PML introduced (and this is mostly my fault, as I
>>>>>>> suggested the buggy solution) is that the vmexit handler from one vcpu
>>>>>>> pauses others to drain the PML queue into the dirty bitmap.  Overall I
>>>>>>> wasn't happy with the design and I've got some ideas to improve it, but
>>>>>>> within the scope of how altp2m was engineered, I proposed
>>>>>>> domain_pause_except_self().
>>>>>>>
>>>>>>> As it turns out, that is vulnerable to deadlocks when you get two vcpus
>>>>>>> trying to pause each other and waiting for each other to become
>>>>>>> de-scheduled.
>>>>>> Makes sense.
>>>>>>
>>>>>>> I see this has been reused by the altp2m code, but it *should* be safe
>>>>>>> to deadlocks now that it takes the hypercall_deadlock_mutext.
>>>>>> Is that already in staging or your x86-next branch? I would like to
>>>>>> verify that the problem is still present or not with that change. I
>>>>>> tested with Xen 4.12 release and that definitely still deadlocks.
>>>>> I don't know if Andrew is talking about this patch (probably not, but
>>>>> it looks at least related):
>>>>>
>>>>> http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=24d5282527f4647907b3572820b5335c15cd0356;hp=29d28b29190ba09d53ae7e475108def84e16e363
>>>>>
>>>> I was referring to 29d28b2919 which is also in 4.12 as it turns out.
>>>> That said, 24d5282527 might in practice be the cause of the deadlock, so
>>>> I'd first experiment with taking that fix out.
>>>>
>>>> I know for certain that it won't be tested with PML enabled, because the
>>>> use of PML is incompatible with write-protecting guest pagetables.
>>>>
>>> Sounds like it's the safe bet to just have PML be disabled for when
>>> introspection is used. I would say it would be even better if the use
>>> of PML could be controlled on a per-guest base instead of the current
>>> global on/off switch. That way it could be disabled only for the
>>> introspected domains.
>>>
>>> I'll do some more experimentation when I get some free time but two
>>> observations that speak against the vCPUs trying to pause each other
>>> being the culprit is that:
>>> - the deadlock doesn't happen with xen-access' altp2m use, it only
>>> happens when there are remapped gfn's in the altp2m views
>>> - I've added a domain_pause/unpause to the PML flusher before it
>>> enters the flush loop but I still got a deadlock
>> Do you have a minimal repro of the deadlock you could share?
> The deadlock is easily reproducible but right now it's only with a
> full setup of DRAKVUF (just the standard setup as described on
> drakvuf.com). Also, the deadlock only triggers after connecting to the
> VM being monitored over VNC (that's what triggers the VGA logdirty
> pages filling up the PML buffer).

Ok - that's probably easy enough to investigate from, when I (or someone
else) can find some copious free time.

~Andrew

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