* [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value
2017-02-13 2:29 [PATCH 0/3] VPMU fixes Boris Ostrovsky
@ 2017-02-13 2:29 ` Boris Ostrovsky
2017-02-13 10:33 ` Andrew Cooper
2017-02-13 12:50 ` Jan Beulich
2017-02-13 2:29 ` [PATCH 2/3] x86/vpmu: Decrement vpmu_count early in vpmu_destroy() Boris Ostrovsky
2017-02-13 2:29 ` [PATCH 3/3] x86: Adjust which files need vpmu.h Boris Ostrovsky
2 siblings, 2 replies; 13+ messages in thread
From: Boris Ostrovsky @ 2017-02-13 2:29 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, Boris Ostrovsky, jbeulich
vpmu_enabled() is currently reported based on VPMU_CONTEXT_ALLOCATED
bit of VPMU's flags. This presents a problem on Intel processors where
VPMU context is allocated lazily, during the first access to VPMU MSRs.
With such delayed allocation we, for example, cannot properly report
CPUID's leaf 0xa since it is likely that the leaf is queried by a
guest before the guest attempts ro read or write the MSR.
Instead of keying vpmu_enabled() off the flags we should compute it
based on vpmu_mode.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
vpmu_enabled() cannot be made an inline in vpmu.h since is_hardware_domain()
is defined in sched.h which includes asm-x86/domain.h which, in turn, needs
vpmu.h
Making it a macro would work but then we'd depend on whoever includes vpmu.h
to also include sched.h and I didn't like that.
xen/arch/x86/cpu/vpmu.c | 6 ++++++
xen/include/asm-x86/vpmu.h | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 35a9403..0252171 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -110,6 +110,12 @@ static void __init parse_vpmu_params(char *s)
printk("VPMU: unknown flags: %s - vpmu disabled!\n", s);
}
+bool_t vpmu_enabled (const struct vcpu *v)
+{
+ return ((vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV)) ||
+ ((vpmu_mode & XENPMU_MODE_ALL) && is_hardware_domain(v->domain)));
+}
+
void vpmu_lvtpc_update(uint32_t val)
{
struct vpmu_struct *vpmu;
diff --git a/xen/include/asm-x86/vpmu.h b/xen/include/asm-x86/vpmu.h
index e50618f..30a0476 100644
--- a/xen/include/asm-x86/vpmu.h
+++ b/xen/include/asm-x86/vpmu.h
@@ -25,7 +25,6 @@
#define vcpu_vpmu(vcpu) (&(vcpu)->arch.vpmu)
#define vpmu_vcpu(vpmu) container_of((vpmu), struct vcpu, arch.vpmu)
-#define vpmu_enabled(vcpu) vpmu_is_set(vcpu_vpmu(vcpu), VPMU_CONTEXT_ALLOCATED)
#define MSR_TYPE_COUNTER 0
#define MSR_TYPE_CTRL 1
@@ -101,6 +100,7 @@ static inline bool_t vpmu_are_all_set(const struct vpmu_struct *vpmu,
return !!((vpmu->flags & mask) == mask);
}
+bool_t vpmu_enabled(const struct vcpu *v);
void vpmu_lvtpc_update(uint32_t val);
int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
uint64_t supported, bool_t is_write);
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value
2017-02-13 2:29 ` [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value Boris Ostrovsky
@ 2017-02-13 10:33 ` Andrew Cooper
2017-02-13 12:50 ` Jan Beulich
1 sibling, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2017-02-13 10:33 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel; +Cc: jbeulich
On 13/02/17 02:29, Boris Ostrovsky wrote:
> vpmu_enabled() is currently reported based on VPMU_CONTEXT_ALLOCATED
> bit of VPMU's flags. This presents a problem on Intel processors where
> VPMU context is allocated lazily, during the first access to VPMU MSRs.
> With such delayed allocation we, for example, cannot properly report
> CPUID's leaf 0xa since it is likely that the leaf is queried by a
> guest before the guest attempts ro read or write the MSR.
>
> Instead of keying vpmu_enabled() off the flags we should compute it
> based on vpmu_mode.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
s/bool_t/bool/
Otherwise, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
I can fix this up on commit if there are no other issues.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value
2017-02-13 2:29 ` [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value Boris Ostrovsky
2017-02-13 10:33 ` Andrew Cooper
@ 2017-02-13 12:50 ` Jan Beulich
2017-02-13 14:38 ` Boris Ostrovsky
1 sibling, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2017-02-13 12:50 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: andrew.cooper3, xen-devel
>>> On 13.02.17 at 03:29, <boris.ostrovsky@oracle.com> wrote:
> vpmu_enabled() is currently reported based on VPMU_CONTEXT_ALLOCATED
> bit of VPMU's flags. This presents a problem on Intel processors where
> VPMU context is allocated lazily, during the first access to VPMU MSRs.
> With such delayed allocation we, for example, cannot properly report
> CPUID's leaf 0xa since it is likely that the leaf is queried by a
> guest before the guest attempts ro read or write the MSR.
>
> Instead of keying vpmu_enabled() off the flags we should compute it
> based on vpmu_mode.
Doesn't this have its own downsides? What if the mode changes
behind the back of a DomU? While certain mode changes are
disallowed while there are active users, there's nothing preventing
mode or features from changing between a DomU querying CPUID
and enabling the vPMU.
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -110,6 +110,12 @@ static void __init parse_vpmu_params(char *s)
> printk("VPMU: unknown flags: %s - vpmu disabled!\n", s);
> }
>
> +bool_t vpmu_enabled (const struct vcpu *v)
Stray blank.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value
2017-02-13 12:50 ` Jan Beulich
@ 2017-02-13 14:38 ` Boris Ostrovsky
2017-02-13 14:44 ` Jan Beulich
0 siblings, 1 reply; 13+ messages in thread
From: Boris Ostrovsky @ 2017-02-13 14:38 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel
On 02/13/2017 07:50 AM, Jan Beulich wrote:
>>>> On 13.02.17 at 03:29, <boris.ostrovsky@oracle.com> wrote:
>> vpmu_enabled() is currently reported based on VPMU_CONTEXT_ALLOCATED
>> bit of VPMU's flags. This presents a problem on Intel processors where
>> VPMU context is allocated lazily, during the first access to VPMU MSRs.
>> With such delayed allocation we, for example, cannot properly report
>> CPUID's leaf 0xa since it is likely that the leaf is queried by a
>> guest before the guest attempts ro read or write the MSR.
>>
>> Instead of keying vpmu_enabled() off the flags we should compute it
>> based on vpmu_mode.
>
> Doesn't this have its own downsides? What if the mode changes
> behind the back of a DomU? While certain mode changes are
> disallowed while there are active users, there's nothing preventing
> mode or features from changing between a DomU querying CPUID
> and enabling the vPMU.
As you said, the mode can't change after a domU VCPU has been
initialized, i.e. before the VCPU started running.
Of course, this was broken, which is what patch 2 (not quite
successfully) tried to fix. And I proposed in my reply to Andrew
to instead define vpmu_enabled() as !!vcpu_vpmu(v)->arch_vpmu_ops.
That should solve problems that both patches try to address.
-boris
>
>> --- a/xen/arch/x86/cpu/vpmu.c
>> +++ b/xen/arch/x86/cpu/vpmu.c
>> @@ -110,6 +110,12 @@ static void __init parse_vpmu_params(char *s)
>> printk("VPMU: unknown flags: %s - vpmu disabled!\n", s);
>> }
>>
>> +bool_t vpmu_enabled (const struct vcpu *v)
>
> Stray blank.
>
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value
2017-02-13 14:38 ` Boris Ostrovsky
@ 2017-02-13 14:44 ` Jan Beulich
2017-02-13 15:02 ` Boris Ostrovsky
0 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2017-02-13 14:44 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: andrew.cooper3, xen-devel
>>> On 13.02.17 at 15:38, <boris.ostrovsky@oracle.com> wrote:
> On 02/13/2017 07:50 AM, Jan Beulich wrote:
>>>>> On 13.02.17 at 03:29, <boris.ostrovsky@oracle.com> wrote:
>>> vpmu_enabled() is currently reported based on VPMU_CONTEXT_ALLOCATED
>>> bit of VPMU's flags. This presents a problem on Intel processors where
>>> VPMU context is allocated lazily, during the first access to VPMU MSRs.
>>> With such delayed allocation we, for example, cannot properly report
>>> CPUID's leaf 0xa since it is likely that the leaf is queried by a
>>> guest before the guest attempts ro read or write the MSR.
>>>
>>> Instead of keying vpmu_enabled() off the flags we should compute it
>>> based on vpmu_mode.
>>
>> Doesn't this have its own downsides? What if the mode changes
>> behind the back of a DomU? While certain mode changes are
>> disallowed while there are active users, there's nothing preventing
>> mode or features from changing between a DomU querying CPUID
>> and enabling the vPMU.
>
> As you said, the mode can't change after a domU VCPU has been
> initialized, i.e. before the VCPU started running.
I don't understand this last part - how is this related to the vCPU
starting to run? Doesn't at least PV initiate the initialization via
hypercall?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value
2017-02-13 14:44 ` Jan Beulich
@ 2017-02-13 15:02 ` Boris Ostrovsky
0 siblings, 0 replies; 13+ messages in thread
From: Boris Ostrovsky @ 2017-02-13 15:02 UTC (permalink / raw)
To: Jan Beulich; +Cc: andrew.cooper3, xen-devel
On 02/13/2017 09:44 AM, Jan Beulich wrote:
>>>> On 13.02.17 at 15:38, <boris.ostrovsky@oracle.com> wrote:
>> On 02/13/2017 07:50 AM, Jan Beulich wrote:
>>>>>> On 13.02.17 at 03:29, <boris.ostrovsky@oracle.com> wrote:
>>>> vpmu_enabled() is currently reported based on VPMU_CONTEXT_ALLOCATED
>>>> bit of VPMU's flags. This presents a problem on Intel processors where
>>>> VPMU context is allocated lazily, during the first access to VPMU MSRs.
>>>> With such delayed allocation we, for example, cannot properly report
>>>> CPUID's leaf 0xa since it is likely that the leaf is queried by a
>>>> guest before the guest attempts ro read or write the MSR.
>>>>
>>>> Instead of keying vpmu_enabled() off the flags we should compute it
>>>> based on vpmu_mode.
>>>
>>> Doesn't this have its own downsides? What if the mode changes
>>> behind the back of a DomU? While certain mode changes are
>>> disallowed while there are active users, there's nothing preventing
>>> mode or features from changing between a DomU querying CPUID
>>> and enabling the vPMU.
>>
>> As you said, the mode can't change after a domU VCPU has been
>> initialized, i.e. before the VCPU started running.
>
> I don't understand this last part - how is this related to the vCPU
> starting to run? Doesn't at least PV initiate the initialization via
> hypercall?
Oh, right. Then looking at arch_vpmu_ops will not work neither for
exactly the same reason --- the hypercall initializing VPMU may come
after CPUID.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] x86/vpmu: Decrement vpmu_count early in vpmu_destroy()
2017-02-13 2:29 [PATCH 0/3] VPMU fixes Boris Ostrovsky
2017-02-13 2:29 ` [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value Boris Ostrovsky
@ 2017-02-13 2:29 ` Boris Ostrovsky
2017-02-13 10:38 ` Andrew Cooper
2017-02-13 2:29 ` [PATCH 3/3] x86: Adjust which files need vpmu.h Boris Ostrovsky
2 siblings, 1 reply; 13+ messages in thread
From: Boris Ostrovsky @ 2017-02-13 2:29 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.cooper3, Boris Ostrovsky, jbeulich
vpmu_count should be decremented even if VPMU_CONTEXT_ALLOCATED
is not set because on Intel processors the context is allocated
lazily and, in fact, might never happen.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
xen/arch/x86/cpu/vpmu.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 0252171..9fa8a18 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -536,6 +536,11 @@ void vpmu_destroy(struct vcpu *v)
{
struct vpmu_struct *vpmu = vcpu_vpmu(v);
+ spin_lock(&vpmu_lock);
+ if ( !is_hardware_domain(v->domain) )
+ vpmu_count--;
+ spin_unlock(&vpmu_lock);
+
if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED) )
return;
@@ -557,11 +562,6 @@ void vpmu_destroy(struct vcpu *v)
vpmu_save_force, v, 1);
vpmu->arch_vpmu_ops->arch_vpmu_destroy(v);
}
-
- spin_lock(&vpmu_lock);
- if ( !is_hardware_domain(v->domain) )
- vpmu_count--;
- spin_unlock(&vpmu_lock);
}
static int pvpmu_init(struct domain *d, xen_pmu_params_t *params)
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/3] x86/vpmu: Decrement vpmu_count early in vpmu_destroy()
2017-02-13 2:29 ` [PATCH 2/3] x86/vpmu: Decrement vpmu_count early in vpmu_destroy() Boris Ostrovsky
@ 2017-02-13 10:38 ` Andrew Cooper
2017-02-13 14:22 ` Boris Ostrovsky
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Cooper @ 2017-02-13 10:38 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel; +Cc: jbeulich
On 13/02/17 02:29, Boris Ostrovsky wrote:
> vpmu_count should be decremented even if VPMU_CONTEXT_ALLOCATED
> is not set because on Intel processors the context is allocated
> lazily and, in fact, might never happen.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
The code in vpmu_initialise() already subtracts 1 from the vpmu_count in
the Intel case.
Won't this now cause an underflow when shutting down a VM which didn't
enable vpmu to start with?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x86/vpmu: Decrement vpmu_count early in vpmu_destroy()
2017-02-13 10:38 ` Andrew Cooper
@ 2017-02-13 14:22 ` Boris Ostrovsky
0 siblings, 0 replies; 13+ messages in thread
From: Boris Ostrovsky @ 2017-02-13 14:22 UTC (permalink / raw)
To: Andrew Cooper, xen-devel; +Cc: jbeulich
On 02/13/2017 05:38 AM, Andrew Cooper wrote:
> On 13/02/17 02:29, Boris Ostrovsky wrote:
>> vpmu_count should be decremented even if VPMU_CONTEXT_ALLOCATED
>> is not set because on Intel processors the context is allocated
>> lazily and, in fact, might never happen.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>
> The code in vpmu_initialise() already subtracts 1 from the vpmu_count in
> the Intel case.
>
> Won't this now cause an underflow when shutting down a VM which didn't
> enable vpmu to start with?
Right.
I think the comment about Intel always needing to initialize VPMU ops is
no longer true so we should only be decrementing the count on error.
But then we'll still need to know whether or not to decrement it in
vpmu_destroy().
How about
#define vpmu_enabled(v) !!vcpu_vpmu(v)->arch_vpmu_ops
and drop the first patch in the series?
I'll add a comment in each vendor's vpmu_initialize() that assignment of
arch_vpmu_ops should be the last thing?
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] x86: Adjust which files need vpmu.h
2017-02-13 2:29 [PATCH 0/3] VPMU fixes Boris Ostrovsky
2017-02-13 2:29 ` [PATCH 1/3] x86/vpmu: Calculate vpmu_enabled() based on vpmu_mode value Boris Ostrovsky
2017-02-13 2:29 ` [PATCH 2/3] x86/vpmu: Decrement vpmu_count early in vpmu_destroy() Boris Ostrovsky
@ 2017-02-13 2:29 ` Boris Ostrovsky
2017-02-13 2:46 ` Tian, Kevin
2017-02-13 10:38 ` Andrew Cooper
2 siblings, 2 replies; 13+ messages in thread
From: Boris Ostrovsky @ 2017-02-13 2:29 UTC (permalink / raw)
To: xen-devel
Cc: Kevin Tian, andrew.cooper3, Boris Ostrovsky, Jun Nakajima, jbeulich
asm-x86/vmcs.h doesn't need it while asm-x86/domain.h does.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
---
xen/include/asm-x86/domain.h | 1 +
xen/include/asm-x86/hvm/vmx/vmcs.h | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index e6c7e13..565823c 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -8,6 +8,7 @@
#include <asm/hvm/domain.h>
#include <asm/e820.h>
#include <asm/mce.h>
+#include <asm/vpmu.h>
#include <asm/x86_emulate.h>
#include <public/vcpu.h>
#include <public/hvm/hvm_info_table.h>
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h
index d71de04..ca5f3d8 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -18,7 +18,6 @@
#ifndef __ASM_X86_HVM_VMX_VMCS_H__
#define __ASM_X86_HVM_VMX_VMCS_H__
-#include <asm/vpmu.h>
#include <asm/hvm/io.h>
#include <irq_vectors.h>
--
1.8.3.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] x86: Adjust which files need vpmu.h
2017-02-13 2:29 ` [PATCH 3/3] x86: Adjust which files need vpmu.h Boris Ostrovsky
@ 2017-02-13 2:46 ` Tian, Kevin
2017-02-13 10:38 ` Andrew Cooper
1 sibling, 0 replies; 13+ messages in thread
From: Tian, Kevin @ 2017-02-13 2:46 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel; +Cc: andrew.cooper3, Nakajima, Jun, jbeulich
> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> Sent: Monday, February 13, 2017 10:30 AM
>
> asm-x86/vmcs.h doesn't need it while asm-x86/domain.h does.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> CC: Jun Nakajima <jun.nakajima@intel.com>
> CC: Kevin Tian <kevin.tian@intel.com>
> ---
>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] x86: Adjust which files need vpmu.h
2017-02-13 2:29 ` [PATCH 3/3] x86: Adjust which files need vpmu.h Boris Ostrovsky
2017-02-13 2:46 ` Tian, Kevin
@ 2017-02-13 10:38 ` Andrew Cooper
1 sibling, 0 replies; 13+ messages in thread
From: Andrew Cooper @ 2017-02-13 10:38 UTC (permalink / raw)
To: Boris Ostrovsky, xen-devel; +Cc: Kevin Tian, Jun Nakajima, jbeulich
On 13/02/17 02:29, Boris Ostrovsky wrote:
> asm-x86/vmcs.h doesn't need it while asm-x86/domain.h does.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread