Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-31 Thread Marc Zyngier
On Wed, 31 Mar 2021 16:28:46 +0100,
Alexandru Elisei  wrote:
> 
> Hello,
> 
> On 3/30/21 12:12 PM, Suzuki K Poulose wrote:
> > Hi Marc
> >
> > On 30/03/2021 11:12, Marc Zyngier wrote:
> >> Hi Suzuki,
> >>
> >> [+ Alex]
> >>
> >> On Tue, 23 Mar 2021 12:06:35 +,
> >> Suzuki K Poulose  wrote:
> >>> [..]
> >>
> >>>   #define MDCR_EL2_TTRF    (1 << 19)
> >>>   #define MDCR_EL2_TPMS    (1 << 14)
> >>>   #define MDCR_EL2_E2PB_MASK    (UL(0x3))
> >>> diff --git a/arch/arm64/include/asm/kvm_host.h
> >>> b/arch/arm64/include/asm/kvm_host.h
> >>> index 3d10e6527f7d..80d0a1a82a4c 100644
> >>> --- a/arch/arm64/include/asm/kvm_host.h
> >>> +++ b/arch/arm64/include/asm/kvm_host.h
> >>> @@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
> >>>   struct kvm_guest_debug_arch regs;
> >>>   /* Statistical profiling extension */
> >>>   u64 pmscr_el1;
> >>> +    /* Self-hosted trace */
> >>> +    u64 trfcr_el1;
> >>>   } host_debug_state;
> >>>     /* VGIC state */
> >>> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> >>> index 5eccbd62fec8..05d25e645b46 100644
> >>> --- a/arch/arm64/kernel/hyp-stub.S
> >>> +++ b/arch/arm64/kernel/hyp-stub.S
> >>> @@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
> >>>   mrs_s    x0, SYS_VBAR_EL12
> >>>   msr    vbar_el1, x0
> >>>   -    // Use EL2 translations for SPE and disable access from EL1
> >>> +    // Use EL2 translations for SPE & TRBE and disable access from EL1
> >>>   mrs    x0, mdcr_el2
> >>>   bic    x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
> >>> +    bic    x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
> >>>   msr    mdcr_el2, x0
> >>>     // Transfer the MM state from EL1 to EL2
> >>> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> >>> index dbc890511631..7b16f42d39f4 100644
> >>> --- a/arch/arm64/kvm/debug.c
> >>> +++ b/arch/arm64/kvm/debug.c
> >>> @@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
> >>>    *  - Debug ROM Address (MDCR_EL2_TDRA)
> >>>    *  - OS related registers (MDCR_EL2_TDOSA)
> >>>    *  - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
> >>> - *  - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
> >>> + *  - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)
> >>
> >> For the record, this is likely to conflict with [1], although that
> >> patch still has some issues.
> >
> > Thanks for the heads up. I think that patch will also conflict with my fixes
> > that is queued in kvmarm/fixes.
> 
> How should I proceed with the patch [1]? For the next iteration, should I 
> rebase
> it on top of kvmarm/fixes or on top of kvmarm/fixes + this series?

On top of kvmarm/fixes, please. I'll merge that branch into 5.13
anyway, as it is taking some time to land upstream.

I'll take care of the conflicts, and shout if I need help!

Thanks,

M.

-- 
Without deviation from the norm, progress is not possible.


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-31 Thread Alexandru Elisei
Hello,

On 3/30/21 12:12 PM, Suzuki K Poulose wrote:
> Hi Marc
>
> On 30/03/2021 11:12, Marc Zyngier wrote:
>> Hi Suzuki,
>>
>> [+ Alex]
>>
>> On Tue, 23 Mar 2021 12:06:35 +,
>> Suzuki K Poulose  wrote:
>>> [..]
>>
>>>   #define MDCR_EL2_TTRF    (1 << 19)
>>>   #define MDCR_EL2_TPMS    (1 << 14)
>>>   #define MDCR_EL2_E2PB_MASK    (UL(0x3))
>>> diff --git a/arch/arm64/include/asm/kvm_host.h
>>> b/arch/arm64/include/asm/kvm_host.h
>>> index 3d10e6527f7d..80d0a1a82a4c 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
>>>   struct kvm_guest_debug_arch regs;
>>>   /* Statistical profiling extension */
>>>   u64 pmscr_el1;
>>> +    /* Self-hosted trace */
>>> +    u64 trfcr_el1;
>>>   } host_debug_state;
>>>     /* VGIC state */
>>> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
>>> index 5eccbd62fec8..05d25e645b46 100644
>>> --- a/arch/arm64/kernel/hyp-stub.S
>>> +++ b/arch/arm64/kernel/hyp-stub.S
>>> @@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
>>>   mrs_s    x0, SYS_VBAR_EL12
>>>   msr    vbar_el1, x0
>>>   -    // Use EL2 translations for SPE and disable access from EL1
>>> +    // Use EL2 translations for SPE & TRBE and disable access from EL1
>>>   mrs    x0, mdcr_el2
>>>   bic    x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
>>> +    bic    x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
>>>   msr    mdcr_el2, x0
>>>     // Transfer the MM state from EL1 to EL2
>>> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
>>> index dbc890511631..7b16f42d39f4 100644
>>> --- a/arch/arm64/kvm/debug.c
>>> +++ b/arch/arm64/kvm/debug.c
>>> @@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
>>>    *  - Debug ROM Address (MDCR_EL2_TDRA)
>>>    *  - OS related registers (MDCR_EL2_TDOSA)
>>>    *  - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
>>> - *  - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
>>> + *  - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)
>>
>> For the record, this is likely to conflict with [1], although that
>> patch still has some issues.
>
> Thanks for the heads up. I think that patch will also conflict with my fixes
> that is queued in kvmarm/fixes.

How should I proceed with the patch [1]? For the next iteration, should I rebase
it on top of kvmarm/fixes or on top of kvmarm/fixes + this series?

[1] https://lore.kernel.org/r/20210323180057.263356-1-alexandru.eli...@arm.com

Thanks,
Alex


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Mathieu Poirier
On Tue, 30 Mar 2021 at 10:47, Greg KH  wrote:
>
> On Tue, Mar 30, 2021 at 10:33:51AM -0600, Mathieu Poirier wrote:
> > On Tue, 30 Mar 2021 at 09:35, Greg KH  wrote:
> > >
> > > On Tue, Mar 30, 2021 at 09:23:14AM -0600, Mathieu Poirier wrote:
> > > > On Tue, Mar 30, 2021 at 11:38:18AM +0100, Suzuki K Poulose wrote:
> > > > > On 26/03/2021 16:55, Mathieu Poirier wrote:
> > > > > > On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:
> > > > > > > For a nvhe host, the EL2 must allow the EL1&0 translation
> > > > > > > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> > > > > > > be saved/restored over a trip to the guest. Also, before
> > > > > > > entering the guest, we must flush any trace data if the
> > > > > > > TRBE was enabled. And we must prohibit the generation
> > > > > > > of trace while we are in EL1 by clearing the TRFCR_EL1.
> > > > > > >
> > > > > > > For vhe, the EL2 must prevent the EL1 access to the Trace
> > > > > > > Buffer.
> > > > > > >
> > > > > > > Cc: Will Deacon 
> > > > > > > Cc: Catalin Marinas 
> > > > > > > Cc: Marc Zyngier 
> > > > > > > Cc: Mark Rutland 
> > > > > > > Cc: Anshuman Khandual 
> > > > > > > Acked-by: Mathieu Poirier 
> > > > > > > Signed-off-by: Suzuki K Poulose 
> > > > > > > ---
> > > > > > >   arch/arm64/include/asm/el2_setup.h | 13 +
> > > > > > >   arch/arm64/include/asm/kvm_arm.h   |  2 ++
> > > > > > >   arch/arm64/include/asm/kvm_host.h  |  2 ++
> > > > > > >   arch/arm64/kernel/hyp-stub.S   |  3 ++-
> > > > > > >   arch/arm64/kvm/debug.c |  6 ++---
> > > > > > >   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 
> > > > > > > ++
> > > > > > >   arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
> > > > > > >   7 files changed, 65 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > >
> > > > > > Marc - do you want me to pick up this one?
> > > > >
> > > > > I think the kvmarm tree is the best route for this patch, given the 
> > > > > amount
> > > > > of changes the tree is going through, in the areas this patch
> > > > > touches. Or else there would be conflicts with merging. And this patch
> > > > > depends on the patches from this series that were queued.
> > > > >
> > > > > Here is the depency tree :
> > > > >
> > > > > a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix 
> > > > > (queued in
> > > > > v5.12-rc3)
> > > > >
> > > > > b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6)
> > > > >
> > > > > c) kvm-arm TRBE host support (Patch 7)
> > > > >
> > > > > d) TRBE driver support (and the ETE changes)
> > > > >
> > > > >
> > > > > (c) code merge depends on -> (a) + (b)
> > > > > (d) build (no conflicts) depends on -> (b)
> > > > >
> > > > >
> > > > > Now (d) has an indirect dependency on (c) for operational correctness 
> > > > > at
> > > > > runtime.
> > > > > So, if :
> > > > >
> > > > > kvmarm tree picks up : b + c
> > > > > coresight tree picksup : b + d
> > > > >
> > > > > and if we could ensure the merge order of the trees are in
> > > > > kvmarm
> > > > > greg-kh (device-misc tree) (coresight goes via this tree)
> > > > >
> > > >
> > > > Greg's char-misc tree is based on the rc releases rather than next.  As 
> > > > such it
> > > > is a while before other branches like kvmarm get merged, causing all 
> > > > sort of
> > > > compilation breakage.
> > >
> > > My tree can not be based on -next, and neither can any other
> > > maintainer's tree, as next is composed of maintainer trees :)
> > >
> >
> > Exactly
> >
> > > > > we should be fine.
> > > > >
> > > > > Additionally, we could rip out the Kconfig changes from the TRBE patch
> > > > > and add it only at the rc1, once we verify both the trees are in to 
> > > > > make
> > > > > sure the runtime operation dependency is not triggered.
> > > > >
> > > >
> > > > We could also do that but Greg might frown at the tactic, and rightly 
> > > > so.  The
> > > > usual way to work with complex merge dependencies is to proceed in 
> > > > steps, which
> > > > would mean that all KVM related patches go in the v5.13 merge window.  
> > > > When that
> > > > is done we add the ETE/TRBE for the v5.14 merge window.  I agree that 
> > > > we waste
> > > > an entire cycle but it guarantees to avoid breaking builds and follows 
> > > > the
> > > > conventional way to do things.
> > >
> > > Or someone creates a single branch with a signed tag and it gets pulled
> > > into multiple maintainer's trees and never rebased.  We've done that
> > > lots of time, nothing new there.  Or everything goes through one tree,
> > > or you wait a release cycle.
> > >
> > > You have 3 choices, pick one :)
> >
> > I'm perfectly happy with getting this entire set merged via Marc's
> > kvmarm tree, as long as you are fine with it.
>
> No objection from me at all for this to go that way.

Swell - Marc, I'll send you a pull request.

>
> thanks,
>
> greg k-h


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Greg KH
On Tue, Mar 30, 2021 at 10:33:51AM -0600, Mathieu Poirier wrote:
> On Tue, 30 Mar 2021 at 09:35, Greg KH  wrote:
> >
> > On Tue, Mar 30, 2021 at 09:23:14AM -0600, Mathieu Poirier wrote:
> > > On Tue, Mar 30, 2021 at 11:38:18AM +0100, Suzuki K Poulose wrote:
> > > > On 26/03/2021 16:55, Mathieu Poirier wrote:
> > > > > On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:
> > > > > > For a nvhe host, the EL2 must allow the EL1&0 translation
> > > > > > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> > > > > > be saved/restored over a trip to the guest. Also, before
> > > > > > entering the guest, we must flush any trace data if the
> > > > > > TRBE was enabled. And we must prohibit the generation
> > > > > > of trace while we are in EL1 by clearing the TRFCR_EL1.
> > > > > >
> > > > > > For vhe, the EL2 must prevent the EL1 access to the Trace
> > > > > > Buffer.
> > > > > >
> > > > > > Cc: Will Deacon 
> > > > > > Cc: Catalin Marinas 
> > > > > > Cc: Marc Zyngier 
> > > > > > Cc: Mark Rutland 
> > > > > > Cc: Anshuman Khandual 
> > > > > > Acked-by: Mathieu Poirier 
> > > > > > Signed-off-by: Suzuki K Poulose 
> > > > > > ---
> > > > > >   arch/arm64/include/asm/el2_setup.h | 13 +
> > > > > >   arch/arm64/include/asm/kvm_arm.h   |  2 ++
> > > > > >   arch/arm64/include/asm/kvm_host.h  |  2 ++
> > > > > >   arch/arm64/kernel/hyp-stub.S   |  3 ++-
> > > > > >   arch/arm64/kvm/debug.c |  6 ++---
> > > > > >   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 
> > > > > > ++
> > > > > >   arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
> > > > > >   7 files changed, 65 insertions(+), 4 deletions(-)
> > > > > >
> > > > >
> > > > > Marc - do you want me to pick up this one?
> > > >
> > > > I think the kvmarm tree is the best route for this patch, given the 
> > > > amount
> > > > of changes the tree is going through, in the areas this patch
> > > > touches. Or else there would be conflicts with merging. And this patch
> > > > depends on the patches from this series that were queued.
> > > >
> > > > Here is the depency tree :
> > > >
> > > > a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix (queued 
> > > > in
> > > > v5.12-rc3)
> > > >
> > > > b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6)
> > > >
> > > > c) kvm-arm TRBE host support (Patch 7)
> > > >
> > > > d) TRBE driver support (and the ETE changes)
> > > >
> > > >
> > > > (c) code merge depends on -> (a) + (b)
> > > > (d) build (no conflicts) depends on -> (b)
> > > >
> > > >
> > > > Now (d) has an indirect dependency on (c) for operational correctness at
> > > > runtime.
> > > > So, if :
> > > >
> > > > kvmarm tree picks up : b + c
> > > > coresight tree picksup : b + d
> > > >
> > > > and if we could ensure the merge order of the trees are in
> > > > kvmarm
> > > > greg-kh (device-misc tree) (coresight goes via this tree)
> > > >
> > >
> > > Greg's char-misc tree is based on the rc releases rather than next.  As 
> > > such it
> > > is a while before other branches like kvmarm get merged, causing all sort 
> > > of
> > > compilation breakage.
> >
> > My tree can not be based on -next, and neither can any other
> > maintainer's tree, as next is composed of maintainer trees :)
> >
> 
> Exactly
> 
> > > > we should be fine.
> > > >
> > > > Additionally, we could rip out the Kconfig changes from the TRBE patch
> > > > and add it only at the rc1, once we verify both the trees are in to make
> > > > sure the runtime operation dependency is not triggered.
> > > >
> > >
> > > We could also do that but Greg might frown at the tactic, and rightly so. 
> > >  The
> > > usual way to work with complex merge dependencies is to proceed in steps, 
> > > which
> > > would mean that all KVM related patches go in the v5.13 merge window.  
> > > When that
> > > is done we add the ETE/TRBE for the v5.14 merge window.  I agree that we 
> > > waste
> > > an entire cycle but it guarantees to avoid breaking builds and follows the
> > > conventional way to do things.
> >
> > Or someone creates a single branch with a signed tag and it gets pulled
> > into multiple maintainer's trees and never rebased.  We've done that
> > lots of time, nothing new there.  Or everything goes through one tree,
> > or you wait a release cycle.
> >
> > You have 3 choices, pick one :)
> 
> I'm perfectly happy with getting this entire set merged via Marc's
> kvmarm tree, as long as you are fine with it.

No objection from me at all for this to go that way.

thanks,

greg k-h


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Mathieu Poirier
On Tue, 30 Mar 2021 at 09:35, Greg KH  wrote:
>
> On Tue, Mar 30, 2021 at 09:23:14AM -0600, Mathieu Poirier wrote:
> > On Tue, Mar 30, 2021 at 11:38:18AM +0100, Suzuki K Poulose wrote:
> > > On 26/03/2021 16:55, Mathieu Poirier wrote:
> > > > On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:
> > > > > For a nvhe host, the EL2 must allow the EL1&0 translation
> > > > > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> > > > > be saved/restored over a trip to the guest. Also, before
> > > > > entering the guest, we must flush any trace data if the
> > > > > TRBE was enabled. And we must prohibit the generation
> > > > > of trace while we are in EL1 by clearing the TRFCR_EL1.
> > > > >
> > > > > For vhe, the EL2 must prevent the EL1 access to the Trace
> > > > > Buffer.
> > > > >
> > > > > Cc: Will Deacon 
> > > > > Cc: Catalin Marinas 
> > > > > Cc: Marc Zyngier 
> > > > > Cc: Mark Rutland 
> > > > > Cc: Anshuman Khandual 
> > > > > Acked-by: Mathieu Poirier 
> > > > > Signed-off-by: Suzuki K Poulose 
> > > > > ---
> > > > >   arch/arm64/include/asm/el2_setup.h | 13 +
> > > > >   arch/arm64/include/asm/kvm_arm.h   |  2 ++
> > > > >   arch/arm64/include/asm/kvm_host.h  |  2 ++
> > > > >   arch/arm64/kernel/hyp-stub.S   |  3 ++-
> > > > >   arch/arm64/kvm/debug.c |  6 ++---
> > > > >   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 
> > > > > ++
> > > > >   arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
> > > > >   7 files changed, 65 insertions(+), 4 deletions(-)
> > > > >
> > > >
> > > > Marc - do you want me to pick up this one?
> > >
> > > I think the kvmarm tree is the best route for this patch, given the amount
> > > of changes the tree is going through, in the areas this patch
> > > touches. Or else there would be conflicts with merging. And this patch
> > > depends on the patches from this series that were queued.
> > >
> > > Here is the depency tree :
> > >
> > > a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix (queued in
> > > v5.12-rc3)
> > >
> > > b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6)
> > >
> > > c) kvm-arm TRBE host support (Patch 7)
> > >
> > > d) TRBE driver support (and the ETE changes)
> > >
> > >
> > > (c) code merge depends on -> (a) + (b)
> > > (d) build (no conflicts) depends on -> (b)
> > >
> > >
> > > Now (d) has an indirect dependency on (c) for operational correctness at
> > > runtime.
> > > So, if :
> > >
> > > kvmarm tree picks up : b + c
> > > coresight tree picksup : b + d
> > >
> > > and if we could ensure the merge order of the trees are in
> > > kvmarm
> > > greg-kh (device-misc tree) (coresight goes via this tree)
> > >
> >
> > Greg's char-misc tree is based on the rc releases rather than next.  As 
> > such it
> > is a while before other branches like kvmarm get merged, causing all sort of
> > compilation breakage.
>
> My tree can not be based on -next, and neither can any other
> maintainer's tree, as next is composed of maintainer trees :)
>

Exactly

> > > we should be fine.
> > >
> > > Additionally, we could rip out the Kconfig changes from the TRBE patch
> > > and add it only at the rc1, once we verify both the trees are in to make
> > > sure the runtime operation dependency is not triggered.
> > >
> >
> > We could also do that but Greg might frown at the tactic, and rightly so.  
> > The
> > usual way to work with complex merge dependencies is to proceed in steps, 
> > which
> > would mean that all KVM related patches go in the v5.13 merge window.  When 
> > that
> > is done we add the ETE/TRBE for the v5.14 merge window.  I agree that we 
> > waste
> > an entire cycle but it guarantees to avoid breaking builds and follows the
> > conventional way to do things.
>
> Or someone creates a single branch with a signed tag and it gets pulled
> into multiple maintainer's trees and never rebased.  We've done that
> lots of time, nothing new there.  Or everything goes through one tree,
> or you wait a release cycle.
>
> You have 3 choices, pick one :)

I'm perfectly happy with getting this entire set merged via Marc's
kvmarm tree, as long as you are fine with it.

>
> thanks,
>
> greg k-h


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Greg KH
On Tue, Mar 30, 2021 at 09:23:14AM -0600, Mathieu Poirier wrote:
> On Tue, Mar 30, 2021 at 11:38:18AM +0100, Suzuki K Poulose wrote:
> > On 26/03/2021 16:55, Mathieu Poirier wrote:
> > > On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:
> > > > For a nvhe host, the EL2 must allow the EL1&0 translation
> > > > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> > > > be saved/restored over a trip to the guest. Also, before
> > > > entering the guest, we must flush any trace data if the
> > > > TRBE was enabled. And we must prohibit the generation
> > > > of trace while we are in EL1 by clearing the TRFCR_EL1.
> > > > 
> > > > For vhe, the EL2 must prevent the EL1 access to the Trace
> > > > Buffer.
> > > > 
> > > > Cc: Will Deacon 
> > > > Cc: Catalin Marinas 
> > > > Cc: Marc Zyngier 
> > > > Cc: Mark Rutland 
> > > > Cc: Anshuman Khandual 
> > > > Acked-by: Mathieu Poirier 
> > > > Signed-off-by: Suzuki K Poulose 
> > > > ---
> > > >   arch/arm64/include/asm/el2_setup.h | 13 +
> > > >   arch/arm64/include/asm/kvm_arm.h   |  2 ++
> > > >   arch/arm64/include/asm/kvm_host.h  |  2 ++
> > > >   arch/arm64/kernel/hyp-stub.S   |  3 ++-
> > > >   arch/arm64/kvm/debug.c |  6 ++---
> > > >   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
> > > >   arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
> > > >   7 files changed, 65 insertions(+), 4 deletions(-)
> > > > 
> > > 
> > > Marc - do you want me to pick up this one?
> > 
> > I think the kvmarm tree is the best route for this patch, given the amount
> > of changes the tree is going through, in the areas this patch
> > touches. Or else there would be conflicts with merging. And this patch
> > depends on the patches from this series that were queued.
> > 
> > Here is the depency tree :
> > 
> > a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix (queued in
> > v5.12-rc3)
> > 
> > b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6)
> > 
> > c) kvm-arm TRBE host support (Patch 7)
> > 
> > d) TRBE driver support (and the ETE changes)
> > 
> > 
> > (c) code merge depends on -> (a) + (b)
> > (d) build (no conflicts) depends on -> (b)
> > 
> > 
> > Now (d) has an indirect dependency on (c) for operational correctness at
> > runtime.
> > So, if :
> > 
> > kvmarm tree picks up : b + c
> > coresight tree picksup : b + d
> > 
> > and if we could ensure the merge order of the trees are in
> > kvmarm
> > greg-kh (device-misc tree) (coresight goes via this tree)
> >
> 
> Greg's char-misc tree is based on the rc releases rather than next.  As such 
> it
> is a while before other branches like kvmarm get merged, causing all sort of
> compilation breakage.

My tree can not be based on -next, and neither can any other
maintainer's tree, as next is composed of maintainer trees :)

> > we should be fine.
> > 
> > Additionally, we could rip out the Kconfig changes from the TRBE patch
> > and add it only at the rc1, once we verify both the trees are in to make
> > sure the runtime operation dependency is not triggered.
> >
> 
> We could also do that but Greg might frown at the tactic, and rightly so.  The
> usual way to work with complex merge dependencies is to proceed in steps, 
> which
> would mean that all KVM related patches go in the v5.13 merge window.  When 
> that
> is done we add the ETE/TRBE for the v5.14 merge window.  I agree that we waste
> an entire cycle but it guarantees to avoid breaking builds and follows the
> conventional way to do things.   

Or someone creates a single branch with a signed tag and it gets pulled
into multiple maintainer's trees and never rebased.  We've done that
lots of time, nothing new there.  Or everything goes through one tree,
or you wait a release cycle.

You have 3 choices, pick one :)

thanks,

greg k-h


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Marc Zyngier
On Tue, 30 Mar 2021 16:23:14 +0100,
Mathieu Poirier  wrote:
> 
> On Tue, Mar 30, 2021 at 11:38:18AM +0100, Suzuki K Poulose wrote:
> > On 26/03/2021 16:55, Mathieu Poirier wrote:
> > > On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:
> > > > For a nvhe host, the EL2 must allow the EL1&0 translation
> > > > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> > > > be saved/restored over a trip to the guest. Also, before
> > > > entering the guest, we must flush any trace data if the
> > > > TRBE was enabled. And we must prohibit the generation
> > > > of trace while we are in EL1 by clearing the TRFCR_EL1.
> > > > 
> > > > For vhe, the EL2 must prevent the EL1 access to the Trace
> > > > Buffer.
> > > > 
> > > > Cc: Will Deacon 
> > > > Cc: Catalin Marinas 
> > > > Cc: Marc Zyngier 
> > > > Cc: Mark Rutland 
> > > > Cc: Anshuman Khandual 
> > > > Acked-by: Mathieu Poirier 
> > > > Signed-off-by: Suzuki K Poulose 
> > > > ---
> > > >   arch/arm64/include/asm/el2_setup.h | 13 +
> > > >   arch/arm64/include/asm/kvm_arm.h   |  2 ++
> > > >   arch/arm64/include/asm/kvm_host.h  |  2 ++
> > > >   arch/arm64/kernel/hyp-stub.S   |  3 ++-
> > > >   arch/arm64/kvm/debug.c |  6 ++---
> > > >   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
> > > >   arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
> > > >   7 files changed, 65 insertions(+), 4 deletions(-)
> > > > 
> > > 
> > > Marc - do you want me to pick up this one?
> > 
> > I think the kvmarm tree is the best route for this patch, given the amount
> > of changes the tree is going through, in the areas this patch
> > touches. Or else there would be conflicts with merging. And this patch
> > depends on the patches from this series that were queued.
> > 
> > Here is the depency tree :
> > 
> > a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix (queued in
> > v5.12-rc3)
> > 
> > b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6)
> > 
> > c) kvm-arm TRBE host support (Patch 7)
> > 
> > d) TRBE driver support (and the ETE changes)
> > 
> > 
> > (c) code merge depends on -> (a) + (b)
> > (d) build (no conflicts) depends on -> (b)
> > 
> > 
> > Now (d) has an indirect dependency on (c) for operational correctness at
> > runtime.
> > So, if :
> > 
> > kvmarm tree picks up : b + c
> > coresight tree picksup : b + d
> > 
> > and if we could ensure the merge order of the trees are in
> > kvmarm
> > greg-kh (device-misc tree) (coresight goes via this tree)
> >
> 
> Greg's char-misc tree is based on the rc releases rather than next.  As such 
> it
> is a while before other branches like kvmarm get merged, causing all sort of
> compilation breakage.
>  
> > we should be fine.
> > 
> > Additionally, we could rip out the Kconfig changes from the TRBE patch
> > and add it only at the rc1, once we verify both the trees are in to make
> > sure the runtime operation dependency is not triggered.
> >
> 
> We could also do that but Greg might frown at the tactic, and
> rightly so.

We do that all the times. Otherwise, it is hardly possible to build an
infrastructure that spans across multiple subsystems *and* involves
userspace. I really wouldn't worry about that.

M.

-- 
Without deviation from the norm, progress is not possible.


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Mathieu Poirier
On Tue, Mar 30, 2021 at 11:38:18AM +0100, Suzuki K Poulose wrote:
> On 26/03/2021 16:55, Mathieu Poirier wrote:
> > On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:
> > > For a nvhe host, the EL2 must allow the EL1&0 translation
> > > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> > > be saved/restored over a trip to the guest. Also, before
> > > entering the guest, we must flush any trace data if the
> > > TRBE was enabled. And we must prohibit the generation
> > > of trace while we are in EL1 by clearing the TRFCR_EL1.
> > > 
> > > For vhe, the EL2 must prevent the EL1 access to the Trace
> > > Buffer.
> > > 
> > > Cc: Will Deacon 
> > > Cc: Catalin Marinas 
> > > Cc: Marc Zyngier 
> > > Cc: Mark Rutland 
> > > Cc: Anshuman Khandual 
> > > Acked-by: Mathieu Poirier 
> > > Signed-off-by: Suzuki K Poulose 
> > > ---
> > >   arch/arm64/include/asm/el2_setup.h | 13 +
> > >   arch/arm64/include/asm/kvm_arm.h   |  2 ++
> > >   arch/arm64/include/asm/kvm_host.h  |  2 ++
> > >   arch/arm64/kernel/hyp-stub.S   |  3 ++-
> > >   arch/arm64/kvm/debug.c |  6 ++---
> > >   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
> > >   arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
> > >   7 files changed, 65 insertions(+), 4 deletions(-)
> > > 
> > 
> > Marc - do you want me to pick up this one?
> 
> I think the kvmarm tree is the best route for this patch, given the amount
> of changes the tree is going through, in the areas this patch
> touches. Or else there would be conflicts with merging. And this patch
> depends on the patches from this series that were queued.
> 
> Here is the depency tree :
> 
> a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix (queued in
> v5.12-rc3)
> 
> b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6)
> 
> c) kvm-arm TRBE host support (Patch 7)
> 
> d) TRBE driver support (and the ETE changes)
> 
> 
> (c) code merge depends on -> (a) + (b)
> (d) build (no conflicts) depends on -> (b)
> 
> 
> Now (d) has an indirect dependency on (c) for operational correctness at
> runtime.
> So, if :
> 
> kvmarm tree picks up : b + c
> coresight tree picksup : b + d
> 
> and if we could ensure the merge order of the trees are in
> kvmarm
> greg-kh (device-misc tree) (coresight goes via this tree)
>

Greg's char-misc tree is based on the rc releases rather than next.  As such it
is a while before other branches like kvmarm get merged, causing all sort of
compilation breakage.
 
> we should be fine.
> 
> Additionally, we could rip out the Kconfig changes from the TRBE patch
> and add it only at the rc1, once we verify both the trees are in to make
> sure the runtime operation dependency is not triggered.
>

We could also do that but Greg might frown at the tactic, and rightly so.  The
usual way to work with complex merge dependencies is to proceed in steps, which
would mean that all KVM related patches go in the v5.13 merge window.  When that
is done we add the ETE/TRBE for the v5.14 merge window.  I agree that we waste
an entire cycle but it guarantees to avoid breaking builds and follows the
conventional way to do things.   

> Thoughts ?
> 
> Suzuki
> 
> > 
> > > diff --git a/arch/arm64/include/asm/el2_setup.h 
> > > b/arch/arm64/include/asm/el2_setup.h
> > > index d77d358f9395..bda918948471 100644
> > > --- a/arch/arm64/include/asm/el2_setup.h
> > > +++ b/arch/arm64/include/asm/el2_setup.h
> > > @@ -65,6 +65,19 @@
> > >   // use EL1&0 
> > > translation.
> > >   .Lskip_spe_\@:
> > > + /* Trace buffer */
> > > + ubfxx0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
> > > + cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not 
> > > present
> > > +
> > > + mrs_s   x0, SYS_TRBIDR_EL1
> > > + and x0, x0, TRBIDR_PROG
> > > + cbnzx0, .Lskip_trace_\@ // If TRBE is available at EL2
> > > +
> > > + mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
> > > + orr x2, x2, x0  // allow the EL1&0 translation
> > > + // to own it.
> > > +
> > > +.Lskip_trace_\@:
> > >   msr mdcr_el2, x2// Configure debug traps
> > >   .endm
> > > diff --git a/arch/arm64/include/asm/kvm_arm.h 
> > > b/arch/arm64/include/asm/kvm_arm.h
> > > index 94d4025acc0b..692c9049befa 100644
> > > --- a/arch/arm64/include/asm/kvm_arm.h
> > > +++ b/arch/arm64/include/asm/kvm_arm.h
> > > @@ -278,6 +278,8 @@
> > >   #define CPTR_EL2_DEFAULTCPTR_EL2_RES1
> > >   /* Hyp Debug Configuration Register bits */
> > > +#define MDCR_EL2_E2TB_MASK   (UL(0x3))
> > > +#define MDCR_EL2_E2TB_SHIFT  (UL(24))
> > >   #define MDCR_EL2_TTRF   (1 << 19)
> > >   #define MDCR_EL2_TPMS   (1 << 14)
> > >   #define MDCR_EL2_E2PB_MASK  (UL(0x3))
> > > diff --git a/arch/arm64/include/asm/kvm_host.h 
> > > b/arch/arm64/include/asm/kvm_host.h
> > > index 

Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Marc Zyngier
On Tue, 30 Mar 2021 14:34:23 +0100,
Suzuki K Poulose  wrote:
> 
> On 30/03/2021 13:15, Marc Zyngier wrote:
> > On Tue, 30 Mar 2021 12:12:49 +0100,
> > Suzuki K Poulose  wrote:

[...]

> >> May be we could do this check at kvm_arch_vcpu_load()/put() ?
> > 
> > That would extend the tracing blackout period enormously, wouldn't it?
> > I'm not sure that's the best thing to do...
> 
> Sorry for not making this clear. We could check if the SPE/TRBE is
> available on this CPU (including the PMB/TRB_IDR bits and a set a flag
> in the VCPU on every kvm_arch_vcpu_load() and cleared on put.
> The actual switching code could check this flag and check if the unit
> is enabled and then do the actual save/restore as we do below.
> (We may be able to even check if unit is enabled there, need to double
> check this.)

Ah, gotcha. Yes, this seems like a reasonable thing to do. We have the
per-vcpu debug flags already, and you could piggy-back on that.

Thanks,

M.

-- 
Without deviation from the norm, progress is not possible.


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Suzuki K Poulose

On 30/03/2021 13:15, Marc Zyngier wrote:

On Tue, 30 Mar 2021 12:12:49 +0100,
Suzuki K Poulose  wrote:


Hi Marc

On 30/03/2021 11:12, Marc Zyngier wrote:

Hi Suzuki,

[+ Alex]

On Tue, 23 Mar 2021 12:06:35 +,
Suzuki K Poulose  wrote:


For a nvhe host, the EL2 must allow the EL1&0 translation
regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
be saved/restored over a trip to the guest. Also, before
entering the guest, we must flush any trace data if the
TRBE was enabled. And we must prohibit the generation
of trace while we are in EL1 by clearing the TRFCR_EL1.

For vhe, the EL2 must prevent the EL1 access to the Trace
Buffer.

Cc: Will Deacon 
Cc: Catalin Marinas 
Cc: Marc Zyngier 
Cc: Mark Rutland 
Cc: Anshuman Khandual 
Acked-by: Mathieu Poirier 
Signed-off-by: Suzuki K Poulose 
---
   arch/arm64/include/asm/el2_setup.h | 13 +
   arch/arm64/include/asm/kvm_arm.h   |  2 ++
   arch/arm64/include/asm/kvm_host.h  |  2 ++
   arch/arm64/kernel/hyp-stub.S   |  3 ++-
   arch/arm64/kvm/debug.c |  6 ++---
   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
   arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
   7 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/el2_setup.h 
b/arch/arm64/include/asm/el2_setup.h
index d77d358f9395..bda918948471 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -65,6 +65,19 @@
// use EL1&0 translation.
 .Lskip_spe_\@:
+   /* Trace buffer */
+   ubfxx0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
+   cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not 
present
+
+   mrs_s   x0, SYS_TRBIDR_EL1
+   and x0, x0, TRBIDR_PROG
+   cbnzx0, .Lskip_trace_\@ // If TRBE is available at EL2
+
+   mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
+   orr x2, x2, x0  // allow the EL1&0 translation
+   // to own it.
+
+.Lskip_trace_\@:
msr mdcr_el2, x2// Configure debug traps
   .endm
   diff --git a/arch/arm64/include/asm/kvm_arm.h
b/arch/arm64/include/asm/kvm_arm.h
index 94d4025acc0b..692c9049befa 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -278,6 +278,8 @@
   #define CPTR_EL2_DEFAULT CPTR_EL2_RES1
 /* Hyp Debug Configuration Register bits */
+#define MDCR_EL2_E2TB_MASK (UL(0x3))
+#define MDCR_EL2_E2TB_SHIFT(UL(24))


Where are these bits defined? DDI0487G_a has them as RES0.


They are part of the Future architecture technology and a register
definition XML is available here :

https://developer.arm.com/documentation/ddi0601/2020-12/AArch64-Registers/MDCR-EL2--Monitor-Debug-Configuration-Register--EL2-?lang=en#fieldset_0-25_24-1



It be worth adding a pointer to that documentation until this is part
of a released ARM ARM.




   #define MDCR_EL2_TTRF(1 << 19)
   #define MDCR_EL2_TPMS(1 << 14)
   #define MDCR_EL2_E2PB_MASK   (UL(0x3))
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 3d10e6527f7d..80d0a1a82a4c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
struct kvm_guest_debug_arch regs;
/* Statistical profiling extension */
u64 pmscr_el1;
+   /* Self-hosted trace */
+   u64 trfcr_el1;
} host_debug_state;
/* VGIC state */
diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index 5eccbd62fec8..05d25e645b46 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
mrs_s   x0, SYS_VBAR_EL12
msr vbar_el1, x0
   -// Use EL2 translations for SPE and disable access from EL1
+   // Use EL2 translations for SPE & TRBE and disable access from EL1
mrs x0, mdcr_el2
bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
+   bic x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
msr mdcr_el2, x0
// Transfer the MM state from EL1 to EL2
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index dbc890511631..7b16f42d39f4 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
*  - Debug ROM Address (MDCR_EL2_TDRA)
*  - OS related registers (MDCR_EL2_TDOSA)
*  - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
- *  - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
+ *  - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)


For the record, this is likely to conflict with [1], although that
patch still has some issues.


Thanks for the heads up. I think that patch will also conflict with my

Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Marc Zyngier
On Tue, 30 Mar 2021 12:12:49 +0100,
Suzuki K Poulose  wrote:
> 
> Hi Marc
> 
> On 30/03/2021 11:12, Marc Zyngier wrote:
> > Hi Suzuki,
> > 
> > [+ Alex]
> > 
> > On Tue, 23 Mar 2021 12:06:35 +,
> > Suzuki K Poulose  wrote:
> >> 
> >> For a nvhe host, the EL2 must allow the EL1&0 translation
> >> regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> >> be saved/restored over a trip to the guest. Also, before
> >> entering the guest, we must flush any trace data if the
> >> TRBE was enabled. And we must prohibit the generation
> >> of trace while we are in EL1 by clearing the TRFCR_EL1.
> >> 
> >> For vhe, the EL2 must prevent the EL1 access to the Trace
> >> Buffer.
> >> 
> >> Cc: Will Deacon 
> >> Cc: Catalin Marinas 
> >> Cc: Marc Zyngier 
> >> Cc: Mark Rutland 
> >> Cc: Anshuman Khandual 
> >> Acked-by: Mathieu Poirier 
> >> Signed-off-by: Suzuki K Poulose 
> >> ---
> >>   arch/arm64/include/asm/el2_setup.h | 13 +
> >>   arch/arm64/include/asm/kvm_arm.h   |  2 ++
> >>   arch/arm64/include/asm/kvm_host.h  |  2 ++
> >>   arch/arm64/kernel/hyp-stub.S   |  3 ++-
> >>   arch/arm64/kvm/debug.c |  6 ++---
> >>   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
> >>   arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
> >>   7 files changed, 65 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/arch/arm64/include/asm/el2_setup.h 
> >> b/arch/arm64/include/asm/el2_setup.h
> >> index d77d358f9395..bda918948471 100644
> >> --- a/arch/arm64/include/asm/el2_setup.h
> >> +++ b/arch/arm64/include/asm/el2_setup.h
> >> @@ -65,6 +65,19 @@
> >>// use EL1&0 translation.
> >> .Lskip_spe_\@:
> >> +  /* Trace buffer */
> >> +  ubfxx0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
> >> +  cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not 
> >> present
> >> +
> >> +  mrs_s   x0, SYS_TRBIDR_EL1
> >> +  and x0, x0, TRBIDR_PROG
> >> +  cbnzx0, .Lskip_trace_\@ // If TRBE is available at EL2
> >> +
> >> +  mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
> >> +  orr x2, x2, x0  // allow the EL1&0 translation
> >> +  // to own it.
> >> +
> >> +.Lskip_trace_\@:
> >>msr mdcr_el2, x2// Configure debug traps
> >>   .endm
> >>   diff --git a/arch/arm64/include/asm/kvm_arm.h
> >> b/arch/arm64/include/asm/kvm_arm.h
> >> index 94d4025acc0b..692c9049befa 100644
> >> --- a/arch/arm64/include/asm/kvm_arm.h
> >> +++ b/arch/arm64/include/asm/kvm_arm.h
> >> @@ -278,6 +278,8 @@
> >>   #define CPTR_EL2_DEFAULT CPTR_EL2_RES1
> >> /* Hyp Debug Configuration Register bits */
> >> +#define MDCR_EL2_E2TB_MASK(UL(0x3))
> >> +#define MDCR_EL2_E2TB_SHIFT   (UL(24))
> > 
> > Where are these bits defined? DDI0487G_a has them as RES0.
> 
> They are part of the Future architecture technology and a register
> definition XML is available here :
> 
> https://developer.arm.com/documentation/ddi0601/2020-12/AArch64-Registers/MDCR-EL2--Monitor-Debug-Configuration-Register--EL2-?lang=en#fieldset_0-25_24-1
>

It be worth adding a pointer to that documentation until this is part
of a released ARM ARM.

> > 
> >>   #define MDCR_EL2_TTRF(1 << 19)
> >>   #define MDCR_EL2_TPMS(1 << 14)
> >>   #define MDCR_EL2_E2PB_MASK   (UL(0x3))
> >> diff --git a/arch/arm64/include/asm/kvm_host.h 
> >> b/arch/arm64/include/asm/kvm_host.h
> >> index 3d10e6527f7d..80d0a1a82a4c 100644
> >> --- a/arch/arm64/include/asm/kvm_host.h
> >> +++ b/arch/arm64/include/asm/kvm_host.h
> >> @@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
> >>struct kvm_guest_debug_arch regs;
> >>/* Statistical profiling extension */
> >>u64 pmscr_el1;
> >> +  /* Self-hosted trace */
> >> +  u64 trfcr_el1;
> >>} host_debug_state;
> >>/* VGIC state */
> >> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> >> index 5eccbd62fec8..05d25e645b46 100644
> >> --- a/arch/arm64/kernel/hyp-stub.S
> >> +++ b/arch/arm64/kernel/hyp-stub.S
> >> @@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
> >>mrs_s   x0, SYS_VBAR_EL12
> >>msr vbar_el1, x0
> >>   -// Use EL2 translations for SPE and disable access from EL1
> >> +  // Use EL2 translations for SPE & TRBE and disable access from EL1
> >>mrs x0, mdcr_el2
> >>bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
> >> +  bic x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
> >>msr mdcr_el2, x0
> >>// Transfer the MM state from EL1 to EL2
> >> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> >> index dbc890511631..7b16f42d39f4 100644
> >> --- a/arch/arm64/kvm/debug.c
> >> +++ b/arch/arm64/kvm/debug.c
> >> @@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
> >>*  - Debug ROM Address (MDCR_EL2_TDRA)
> >>*  - OS related 

Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Suzuki K Poulose

Hi Marc

On 30/03/2021 11:12, Marc Zyngier wrote:

Hi Suzuki,

[+ Alex]

On Tue, 23 Mar 2021 12:06:35 +,
Suzuki K Poulose  wrote:


For a nvhe host, the EL2 must allow the EL1&0 translation
regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
be saved/restored over a trip to the guest. Also, before
entering the guest, we must flush any trace data if the
TRBE was enabled. And we must prohibit the generation
of trace while we are in EL1 by clearing the TRFCR_EL1.

For vhe, the EL2 must prevent the EL1 access to the Trace
Buffer.

Cc: Will Deacon 
Cc: Catalin Marinas 
Cc: Marc Zyngier 
Cc: Mark Rutland 
Cc: Anshuman Khandual 
Acked-by: Mathieu Poirier 
Signed-off-by: Suzuki K Poulose 
---
  arch/arm64/include/asm/el2_setup.h | 13 +
  arch/arm64/include/asm/kvm_arm.h   |  2 ++
  arch/arm64/include/asm/kvm_host.h  |  2 ++
  arch/arm64/kernel/hyp-stub.S   |  3 ++-
  arch/arm64/kvm/debug.c |  6 ++---
  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
  arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
  7 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/el2_setup.h 
b/arch/arm64/include/asm/el2_setup.h
index d77d358f9395..bda918948471 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -65,6 +65,19 @@
// use EL1&0 translation.
  
  .Lskip_spe_\@:

+   /* Trace buffer */
+   ubfxx0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
+   cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not 
present
+
+   mrs_s   x0, SYS_TRBIDR_EL1
+   and x0, x0, TRBIDR_PROG
+   cbnzx0, .Lskip_trace_\@ // If TRBE is available at EL2
+
+   mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
+   orr x2, x2, x0  // allow the EL1&0 translation
+   // to own it.
+
+.Lskip_trace_\@:
msr mdcr_el2, x2// Configure debug traps
  .endm
  
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h

index 94d4025acc0b..692c9049befa 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -278,6 +278,8 @@
  #define CPTR_EL2_DEFAULT  CPTR_EL2_RES1
  
  /* Hyp Debug Configuration Register bits */

+#define MDCR_EL2_E2TB_MASK (UL(0x3))
+#define MDCR_EL2_E2TB_SHIFT(UL(24))


Where are these bits defined? DDI0487G_a has them as RES0.


They are part of the Future architecture technology and a register 
definition XML is available here :


https://developer.arm.com/documentation/ddi0601/2020-12/AArch64-Registers/MDCR-EL2--Monitor-Debug-Configuration-Register--EL2-?lang=en#fieldset_0-25_24-1




  #define MDCR_EL2_TTRF (1 << 19)
  #define MDCR_EL2_TPMS (1 << 14)
  #define MDCR_EL2_E2PB_MASK(UL(0x3))
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 3d10e6527f7d..80d0a1a82a4c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
struct kvm_guest_debug_arch regs;
/* Statistical profiling extension */
u64 pmscr_el1;
+   /* Self-hosted trace */
+   u64 trfcr_el1;
} host_debug_state;
  
  	/* VGIC state */

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index 5eccbd62fec8..05d25e645b46 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
mrs_s   x0, SYS_VBAR_EL12
msr vbar_el1, x0
  
-	// Use EL2 translations for SPE and disable access from EL1

+   // Use EL2 translations for SPE & TRBE and disable access from EL1
mrs x0, mdcr_el2
bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
+   bic x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
msr mdcr_el2, x0
  
  	// Transfer the MM state from EL1 to EL2

diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index dbc890511631..7b16f42d39f4 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
   *  - Debug ROM Address (MDCR_EL2_TDRA)
   *  - OS related registers (MDCR_EL2_TDOSA)
   *  - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
- *  - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
+ *  - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)


For the record, this is likely to conflict with [1], although that
patch still has some issues.


Thanks for the heads up. I think that patch will also conflict with my 
fixes that is queued in kvmarm/fixes.






   *
   * Additionally, KVM only traps guest accesses to the debug registers if
   * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
@@ -107,8 +107,8 @@ void 

Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Suzuki K Poulose

On 26/03/2021 16:55, Mathieu Poirier wrote:

On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:

For a nvhe host, the EL2 must allow the EL1&0 translation
regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
be saved/restored over a trip to the guest. Also, before
entering the guest, we must flush any trace data if the
TRBE was enabled. And we must prohibit the generation
of trace while we are in EL1 by clearing the TRFCR_EL1.

For vhe, the EL2 must prevent the EL1 access to the Trace
Buffer.

Cc: Will Deacon 
Cc: Catalin Marinas 
Cc: Marc Zyngier 
Cc: Mark Rutland 
Cc: Anshuman Khandual 
Acked-by: Mathieu Poirier 
Signed-off-by: Suzuki K Poulose 
---
  arch/arm64/include/asm/el2_setup.h | 13 +
  arch/arm64/include/asm/kvm_arm.h   |  2 ++
  arch/arm64/include/asm/kvm_host.h  |  2 ++
  arch/arm64/kernel/hyp-stub.S   |  3 ++-
  arch/arm64/kvm/debug.c |  6 ++---
  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
  arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
  7 files changed, 65 insertions(+), 4 deletions(-)



Marc - do you want me to pick up this one?


I think the kvmarm tree is the best route for this patch, given the 
amount of changes the tree is going through, in the areas this patch

touches. Or else there would be conflicts with merging. And this patch
depends on the patches from this series that were queued.

Here is the depency tree :

a) kvm-arm fixes for debug (Patch 1, 2) & SPE save-restore fix (queued 
in v5.12-rc3)


b) TRBE defintions and Trace synchronization barrier (Patches 5 & 6)

c) kvm-arm TRBE host support (Patch 7)

d) TRBE driver support (and the ETE changes)


(c) code merge depends on -> (a) + (b)
(d) build (no conflicts) depends on -> (b)


Now (d) has an indirect dependency on (c) for operational correctness at
runtime.
So, if :

kvmarm tree picks up : b + c
coresight tree picksup : b + d

and if we could ensure the merge order of the trees are in
kvmarm
greg-kh (device-misc tree) (coresight goes via this tree)

we should be fine.

Additionally, we could rip out the Kconfig changes from the TRBE patch
and add it only at the rc1, once we verify both the trees are in to make
sure the runtime operation dependency is not triggered.

Thoughts ?

Suzuki




diff --git a/arch/arm64/include/asm/el2_setup.h 
b/arch/arm64/include/asm/el2_setup.h
index d77d358f9395..bda918948471 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -65,6 +65,19 @@
// use EL1&0 translation.
  
  .Lskip_spe_\@:

+   /* Trace buffer */
+   ubfxx0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
+   cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not 
present
+
+   mrs_s   x0, SYS_TRBIDR_EL1
+   and x0, x0, TRBIDR_PROG
+   cbnzx0, .Lskip_trace_\@ // If TRBE is available at EL2
+
+   mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
+   orr x2, x2, x0  // allow the EL1&0 translation
+   // to own it.
+
+.Lskip_trace_\@:
msr mdcr_el2, x2// Configure debug traps
  .endm
  
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h

index 94d4025acc0b..692c9049befa 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -278,6 +278,8 @@
  #define CPTR_EL2_DEFAULT  CPTR_EL2_RES1
  
  /* Hyp Debug Configuration Register bits */

+#define MDCR_EL2_E2TB_MASK (UL(0x3))
+#define MDCR_EL2_E2TB_SHIFT(UL(24))
  #define MDCR_EL2_TTRF (1 << 19)
  #define MDCR_EL2_TPMS (1 << 14)
  #define MDCR_EL2_E2PB_MASK(UL(0x3))
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 3d10e6527f7d..80d0a1a82a4c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
struct kvm_guest_debug_arch regs;
/* Statistical profiling extension */
u64 pmscr_el1;
+   /* Self-hosted trace */
+   u64 trfcr_el1;
} host_debug_state;
  
  	/* VGIC state */

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index 5eccbd62fec8..05d25e645b46 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
mrs_s   x0, SYS_VBAR_EL12
msr vbar_el1, x0
  
-	// Use EL2 translations for SPE and disable access from EL1

+   // Use EL2 translations for SPE & TRBE and disable access from EL1
mrs x0, mdcr_el2
bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
+   bic x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
msr mdcr_el2, x0
  
  	// Transfer the MM state from EL1 to EL2

diff --git a/arch/arm64/kvm/debug.c 

Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Marc Zyngier
Hi Mathieu,

On Fri, 26 Mar 2021 16:55:50 +,
Mathieu Poirier  wrote:
> 
> On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:
> > For a nvhe host, the EL2 must allow the EL1&0 translation
> > regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> > be saved/restored over a trip to the guest. Also, before
> > entering the guest, we must flush any trace data if the
> > TRBE was enabled. And we must prohibit the generation
> > of trace while we are in EL1 by clearing the TRFCR_EL1.
> > 
> > For vhe, the EL2 must prevent the EL1 access to the Trace
> > Buffer.
> > 
> > Cc: Will Deacon 
> > Cc: Catalin Marinas 
> > Cc: Marc Zyngier 
> > Cc: Mark Rutland 
> > Cc: Anshuman Khandual 
> > Acked-by: Mathieu Poirier 
> > Signed-off-by: Suzuki K Poulose 
> > ---
> >  arch/arm64/include/asm/el2_setup.h | 13 +
> >  arch/arm64/include/asm/kvm_arm.h   |  2 ++
> >  arch/arm64/include/asm/kvm_host.h  |  2 ++
> >  arch/arm64/kernel/hyp-stub.S   |  3 ++-
> >  arch/arm64/kvm/debug.c |  6 ++---
> >  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
> >  arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
> >  7 files changed, 65 insertions(+), 4 deletions(-)
> > 
> 
> Marc - do you want me to pick up this one?

I just went through the KVM patch, and I have a couple of question
that Suzuki can hopefully address quickly enough. As for merging it
via your tree, I'm worried that it will conflict with other patches
that are in flight.

We can hopefully set up a stable branch between the two trees.

Thanks,

M.

-- 
Without deviation from the norm, progress is not possible.


Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-30 Thread Marc Zyngier
Hi Suzuki,

[+ Alex]

On Tue, 23 Mar 2021 12:06:35 +,
Suzuki K Poulose  wrote:
> 
> For a nvhe host, the EL2 must allow the EL1&0 translation
> regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> be saved/restored over a trip to the guest. Also, before
> entering the guest, we must flush any trace data if the
> TRBE was enabled. And we must prohibit the generation
> of trace while we are in EL1 by clearing the TRFCR_EL1.
> 
> For vhe, the EL2 must prevent the EL1 access to the Trace
> Buffer.
> 
> Cc: Will Deacon 
> Cc: Catalin Marinas 
> Cc: Marc Zyngier 
> Cc: Mark Rutland 
> Cc: Anshuman Khandual 
> Acked-by: Mathieu Poirier 
> Signed-off-by: Suzuki K Poulose 
> ---
>  arch/arm64/include/asm/el2_setup.h | 13 +
>  arch/arm64/include/asm/kvm_arm.h   |  2 ++
>  arch/arm64/include/asm/kvm_host.h  |  2 ++
>  arch/arm64/kernel/hyp-stub.S   |  3 ++-
>  arch/arm64/kvm/debug.c |  6 ++---
>  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
>  arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
>  7 files changed, 65 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/el2_setup.h 
> b/arch/arm64/include/asm/el2_setup.h
> index d77d358f9395..bda918948471 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -65,6 +65,19 @@
>   // use EL1&0 translation.
>  
>  .Lskip_spe_\@:
> + /* Trace buffer */
> + ubfxx0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
> + cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not 
> present
> +
> + mrs_s   x0, SYS_TRBIDR_EL1
> + and x0, x0, TRBIDR_PROG
> + cbnzx0, .Lskip_trace_\@ // If TRBE is available at EL2
> +
> + mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
> + orr x2, x2, x0  // allow the EL1&0 translation
> + // to own it.
> +
> +.Lskip_trace_\@:
>   msr mdcr_el2, x2// Configure debug traps
>  .endm
>  
> diff --git a/arch/arm64/include/asm/kvm_arm.h 
> b/arch/arm64/include/asm/kvm_arm.h
> index 94d4025acc0b..692c9049befa 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -278,6 +278,8 @@
>  #define CPTR_EL2_DEFAULT CPTR_EL2_RES1
>  
>  /* Hyp Debug Configuration Register bits */
> +#define MDCR_EL2_E2TB_MASK   (UL(0x3))
> +#define MDCR_EL2_E2TB_SHIFT  (UL(24))

Where are these bits defined? DDI0487G_a has them as RES0.

>  #define MDCR_EL2_TTRF(1 << 19)
>  #define MDCR_EL2_TPMS(1 << 14)
>  #define MDCR_EL2_E2PB_MASK   (UL(0x3))
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index 3d10e6527f7d..80d0a1a82a4c 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
>   struct kvm_guest_debug_arch regs;
>   /* Statistical profiling extension */
>   u64 pmscr_el1;
> + /* Self-hosted trace */
> + u64 trfcr_el1;
>   } host_debug_state;
>  
>   /* VGIC state */
> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> index 5eccbd62fec8..05d25e645b46 100644
> --- a/arch/arm64/kernel/hyp-stub.S
> +++ b/arch/arm64/kernel/hyp-stub.S
> @@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
>   mrs_s   x0, SYS_VBAR_EL12
>   msr vbar_el1, x0
>  
> - // Use EL2 translations for SPE and disable access from EL1
> + // Use EL2 translations for SPE & TRBE and disable access from EL1
>   mrs x0, mdcr_el2
>   bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
> + bic x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
>   msr mdcr_el2, x0
>  
>   // Transfer the MM state from EL1 to EL2
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index dbc890511631..7b16f42d39f4 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
>   *  - Debug ROM Address (MDCR_EL2_TDRA)
>   *  - OS related registers (MDCR_EL2_TDOSA)
>   *  - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
> - *  - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
> + *  - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)

For the record, this is likely to conflict with [1], although that
patch still has some issues.

>   *
>   * Additionally, KVM only traps guest accesses to the debug registers if
>   * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
> @@ -107,8 +107,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
>   trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
>  
>   /*
> -  * This also clears MDCR_EL2_E2PB_MASK to disable guest access
> -  * to the profiling buffer.
> +  * This also clears MDCR_EL2_E2PB_MASK and 

Re: [PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-26 Thread Mathieu Poirier
On Tue, Mar 23, 2021 at 12:06:35PM +, Suzuki K Poulose wrote:
> For a nvhe host, the EL2 must allow the EL1&0 translation
> regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
> be saved/restored over a trip to the guest. Also, before
> entering the guest, we must flush any trace data if the
> TRBE was enabled. And we must prohibit the generation
> of trace while we are in EL1 by clearing the TRFCR_EL1.
> 
> For vhe, the EL2 must prevent the EL1 access to the Trace
> Buffer.
> 
> Cc: Will Deacon 
> Cc: Catalin Marinas 
> Cc: Marc Zyngier 
> Cc: Mark Rutland 
> Cc: Anshuman Khandual 
> Acked-by: Mathieu Poirier 
> Signed-off-by: Suzuki K Poulose 
> ---
>  arch/arm64/include/asm/el2_setup.h | 13 +
>  arch/arm64/include/asm/kvm_arm.h   |  2 ++
>  arch/arm64/include/asm/kvm_host.h  |  2 ++
>  arch/arm64/kernel/hyp-stub.S   |  3 ++-
>  arch/arm64/kvm/debug.c |  6 ++---
>  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
>  arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
>  7 files changed, 65 insertions(+), 4 deletions(-)
> 

Marc - do you want me to pick up this one?

> diff --git a/arch/arm64/include/asm/el2_setup.h 
> b/arch/arm64/include/asm/el2_setup.h
> index d77d358f9395..bda918948471 100644
> --- a/arch/arm64/include/asm/el2_setup.h
> +++ b/arch/arm64/include/asm/el2_setup.h
> @@ -65,6 +65,19 @@
>   // use EL1&0 translation.
>  
>  .Lskip_spe_\@:
> + /* Trace buffer */
> + ubfxx0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
> + cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not 
> present
> +
> + mrs_s   x0, SYS_TRBIDR_EL1
> + and x0, x0, TRBIDR_PROG
> + cbnzx0, .Lskip_trace_\@ // If TRBE is available at EL2
> +
> + mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
> + orr x2, x2, x0  // allow the EL1&0 translation
> + // to own it.
> +
> +.Lskip_trace_\@:
>   msr mdcr_el2, x2// Configure debug traps
>  .endm
>  
> diff --git a/arch/arm64/include/asm/kvm_arm.h 
> b/arch/arm64/include/asm/kvm_arm.h
> index 94d4025acc0b..692c9049befa 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -278,6 +278,8 @@
>  #define CPTR_EL2_DEFAULT CPTR_EL2_RES1
>  
>  /* Hyp Debug Configuration Register bits */
> +#define MDCR_EL2_E2TB_MASK   (UL(0x3))
> +#define MDCR_EL2_E2TB_SHIFT  (UL(24))
>  #define MDCR_EL2_TTRF(1 << 19)
>  #define MDCR_EL2_TPMS(1 << 14)
>  #define MDCR_EL2_E2PB_MASK   (UL(0x3))
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index 3d10e6527f7d..80d0a1a82a4c 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
>   struct kvm_guest_debug_arch regs;
>   /* Statistical profiling extension */
>   u64 pmscr_el1;
> + /* Self-hosted trace */
> + u64 trfcr_el1;
>   } host_debug_state;
>  
>   /* VGIC state */
> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> index 5eccbd62fec8..05d25e645b46 100644
> --- a/arch/arm64/kernel/hyp-stub.S
> +++ b/arch/arm64/kernel/hyp-stub.S
> @@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
>   mrs_s   x0, SYS_VBAR_EL12
>   msr vbar_el1, x0
>  
> - // Use EL2 translations for SPE and disable access from EL1
> + // Use EL2 translations for SPE & TRBE and disable access from EL1
>   mrs x0, mdcr_el2
>   bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
> + bic x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
>   msr mdcr_el2, x0
>  
>   // Transfer the MM state from EL1 to EL2
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index dbc890511631..7b16f42d39f4 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
>   *  - Debug ROM Address (MDCR_EL2_TDRA)
>   *  - OS related registers (MDCR_EL2_TDOSA)
>   *  - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
> - *  - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
> + *  - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)
>   *
>   * Additionally, KVM only traps guest accesses to the debug registers if
>   * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
> @@ -107,8 +107,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
>   trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
>  
>   /*
> -  * This also clears MDCR_EL2_E2PB_MASK to disable guest access
> -  * to the profiling buffer.
> +  * This also clears MDCR_EL2_E2PB_MASK and MDCR_EL2_E2TB_MASK
> +  * to disable guest access to the profiling and trace buffers
>*/
>   vcpu->arch.mdcr_el2 = 

[PATCH v5 07/19] arm64: kvm: Enable access to TRBE support for host

2021-03-23 Thread Suzuki K Poulose
For a nvhe host, the EL2 must allow the EL1&0 translation
regime for TraceBuffer (MDCR_EL2.E2TB == 0b11). This must
be saved/restored over a trip to the guest. Also, before
entering the guest, we must flush any trace data if the
TRBE was enabled. And we must prohibit the generation
of trace while we are in EL1 by clearing the TRFCR_EL1.

For vhe, the EL2 must prevent the EL1 access to the Trace
Buffer.

Cc: Will Deacon 
Cc: Catalin Marinas 
Cc: Marc Zyngier 
Cc: Mark Rutland 
Cc: Anshuman Khandual 
Acked-by: Mathieu Poirier 
Signed-off-by: Suzuki K Poulose 
---
 arch/arm64/include/asm/el2_setup.h | 13 +
 arch/arm64/include/asm/kvm_arm.h   |  2 ++
 arch/arm64/include/asm/kvm_host.h  |  2 ++
 arch/arm64/kernel/hyp-stub.S   |  3 ++-
 arch/arm64/kvm/debug.c |  6 ++---
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 42 ++
 arch/arm64/kvm/hyp/nvhe/switch.c   |  1 +
 7 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/el2_setup.h 
b/arch/arm64/include/asm/el2_setup.h
index d77d358f9395..bda918948471 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -65,6 +65,19 @@
// use EL1&0 translation.
 
 .Lskip_spe_\@:
+   /* Trace buffer */
+   ubfxx0, x1, #ID_AA64DFR0_TRBE_SHIFT, #4
+   cbz x0, .Lskip_trace_\@ // Skip if TraceBuffer is not 
present
+
+   mrs_s   x0, SYS_TRBIDR_EL1
+   and x0, x0, TRBIDR_PROG
+   cbnzx0, .Lskip_trace_\@ // If TRBE is available at EL2
+
+   mov x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
+   orr x2, x2, x0  // allow the EL1&0 translation
+   // to own it.
+
+.Lskip_trace_\@:
msr mdcr_el2, x2// Configure debug traps
 .endm
 
diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 94d4025acc0b..692c9049befa 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -278,6 +278,8 @@
 #define CPTR_EL2_DEFAULT   CPTR_EL2_RES1
 
 /* Hyp Debug Configuration Register bits */
+#define MDCR_EL2_E2TB_MASK (UL(0x3))
+#define MDCR_EL2_E2TB_SHIFT(UL(24))
 #define MDCR_EL2_TTRF  (1 << 19)
 #define MDCR_EL2_TPMS  (1 << 14)
 #define MDCR_EL2_E2PB_MASK (UL(0x3))
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index 3d10e6527f7d..80d0a1a82a4c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -315,6 +315,8 @@ struct kvm_vcpu_arch {
struct kvm_guest_debug_arch regs;
/* Statistical profiling extension */
u64 pmscr_el1;
+   /* Self-hosted trace */
+   u64 trfcr_el1;
} host_debug_state;
 
/* VGIC state */
diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index 5eccbd62fec8..05d25e645b46 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -115,9 +115,10 @@ SYM_CODE_START_LOCAL(mutate_to_vhe)
mrs_s   x0, SYS_VBAR_EL12
msr vbar_el1, x0
 
-   // Use EL2 translations for SPE and disable access from EL1
+   // Use EL2 translations for SPE & TRBE and disable access from EL1
mrs x0, mdcr_el2
bic x0, x0, #(MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)
+   bic x0, x0, #(MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT)
msr mdcr_el2, x0
 
// Transfer the MM state from EL1 to EL2
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index dbc890511631..7b16f42d39f4 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -89,7 +89,7 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
  *  - Debug ROM Address (MDCR_EL2_TDRA)
  *  - OS related registers (MDCR_EL2_TDOSA)
  *  - Statistical profiler (MDCR_EL2_TPMS/MDCR_EL2_E2PB)
- *  - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
+ *  - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)
  *
  * Additionally, KVM only traps guest accesses to the debug registers if
  * the guest is not actively using them (see the KVM_ARM64_DEBUG_DIRTY
@@ -107,8 +107,8 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
 
/*
-* This also clears MDCR_EL2_E2PB_MASK to disable guest access
-* to the profiling buffer.
+* This also clears MDCR_EL2_E2PB_MASK and MDCR_EL2_E2TB_MASK
+* to disable guest access to the profiling and trace buffers
 */
vcpu->arch.mdcr_el2 = __this_cpu_read(mdcr_el2) & MDCR_EL2_HPMN_MASK;
vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM |
diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c 
b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
index f401724f12ef..9499e18dd28f 100644
--- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
+++