2021-03-23 12:08:56

by Suzuki K Poulose

[permalink] [raw]
Subject: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

tsb csync synchronizes the trace operation of instructions.
The instruction is a nop when FEAT_TRF is not implemented.

Cc: Mathieu Poirier <[email protected]>
Cc: Mike Leach <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
arch/arm64/include/asm/barrier.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index c3009b0e5239..5a8367a2b868 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -23,6 +23,7 @@
#define dsb(opt) asm volatile("dsb " #opt : : : "memory")

#define psb_csync() asm volatile("hint #17" : : : "memory")
+#define tsb_csync() asm volatile("hint #18" : : : "memory")
#define csdb() asm volatile("hint #20" : : : "memory")

#define spec_bar() asm volatile(ALTERNATIVE("dsb nsh\nisb\n", \
--
2.24.1


2021-03-23 18:26:10

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

Hi Suzuki?

On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
> tsb csync synchronizes the trace operation of instructions.
> The instruction is a nop when FEAT_TRF is not implemented.
>
> Cc: Mathieu Poirier <[email protected]>
> Cc: Mike Leach <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Will Deacon <[email protected]>
> Signed-off-by: Suzuki K Poulose <[email protected]>

How do you plan to merge these patches? If they go via the coresight
tree:

Acked-by: Catalin Marinas <[email protected]>

2021-03-24 13:52:02

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On Wed, 24 Mar 2021 09:39:13 +0000,
Suzuki K Poulose <[email protected]> wrote:
>
> On 23/03/2021 18:21, Catalin Marinas wrote:
> > Hi Suzuki?
> >
> > On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
> >> tsb csync synchronizes the trace operation of instructions.
> >> The instruction is a nop when FEAT_TRF is not implemented.
> >>
> >> Cc: Mathieu Poirier <[email protected]>
> >> Cc: Mike Leach <[email protected]>
> >> Cc: Catalin Marinas <[email protected]>
> >> Cc: Will Deacon <[email protected]>
> >> Signed-off-by: Suzuki K Poulose <[email protected]>
> >
> > How do you plan to merge these patches? If they go via the coresight
> > tree:
> >
>
> Ideally all of this should go via the CoreSight tree to have the
> dependencies solved at one place. But there are some issues :
>
> If this makes to 5.13 queue for CoreSight,
>
> 1) CoreSight next is based on rc2 at the moment and we have fixes gone
> into rc3 and later, which this series will depend on. (We could move
> the next tree forward to a later rc to solve this).
>
> 2) There could be conflicts with the kvmarm tree for the KVM host
> changes (That has dependency on the TRBE definitions patch).
>
> If it doesn't make to 5.13 queue, it would be good to have this patch,
> the TRBE defintions and the KVM host patches queued for 5.13 (not sure
> if this is acceptable) and we could rebase the CoreSight changes on 5.13
> and push it to next release.
>
> I am open for other suggestions.
>
> Marc, Mathieu,
>
> Thoughts ?

I was planning to take the first two patches in 5.12 as fixes (they
are queued already, and would hopefully land in -rc5). If that doesn't
fit with the plan, please let me know ASAP.

Thanks,

M.

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

2021-03-24 15:56:27

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On 24/03/2021 13:49, Marc Zyngier wrote:
> On Wed, 24 Mar 2021 09:39:13 +0000,
> Suzuki K Poulose <[email protected]> wrote:
>>
>> On 23/03/2021 18:21, Catalin Marinas wrote:
>>> Hi Suzuki?
>>>
>>> On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
>>>> tsb csync synchronizes the trace operation of instructions.
>>>> The instruction is a nop when FEAT_TRF is not implemented.
>>>>
>>>> Cc: Mathieu Poirier <[email protected]>
>>>> Cc: Mike Leach <[email protected]>
>>>> Cc: Catalin Marinas <[email protected]>
>>>> Cc: Will Deacon <[email protected]>
>>>> Signed-off-by: Suzuki K Poulose <[email protected]>
>>>
>>> How do you plan to merge these patches? If they go via the coresight
>>> tree:
>>>
>>
>> Ideally all of this should go via the CoreSight tree to have the
>> dependencies solved at one place. But there are some issues :
>>
>> If this makes to 5.13 queue for CoreSight,
>>
>> 1) CoreSight next is based on rc2 at the moment and we have fixes gone
>> into rc3 and later, which this series will depend on. (We could move
>> the next tree forward to a later rc to solve this).
>>
>> 2) There could be conflicts with the kvmarm tree for the KVM host
>> changes (That has dependency on the TRBE definitions patch).
>>
>> If it doesn't make to 5.13 queue, it would be good to have this patch,
>> the TRBE defintions and the KVM host patches queued for 5.13 (not sure
>> if this is acceptable) and we could rebase the CoreSight changes on 5.13
>> and push it to next release.
>>
>> I am open for other suggestions.
>>
>> Marc, Mathieu,
>>
>> Thoughts ?
>
> I was planning to take the first two patches in 5.12 as fixes (they
> are queued already, and would hopefully land in -rc5). If that doesn't
> fit with the plan, please let me know ASAP.

Marc,

I think it would be better to hold on pushing those patches until we
have a clarity on how things will go.

Sorry for the confusion.

Kind regards
Suzuki

>
> Thanks,
>
> M.
>

2021-03-24 17:13:08

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On 24/03/2021 16:30, Marc Zyngier wrote:
> On Wed, 24 Mar 2021 16:25:12 +0000,
> Suzuki K Poulose <[email protected]> wrote:
>>
>> On 24/03/2021 16:16, Marc Zyngier wrote:
>>> On Wed, 24 Mar 2021 15:51:14 +0000,
>>> Suzuki K Poulose <[email protected]> wrote:
>>>>
>>>> On 24/03/2021 13:49, Marc Zyngier wrote:
>>>>> On Wed, 24 Mar 2021 09:39:13 +0000,
>>>>> Suzuki K Poulose <[email protected]> wrote:
>>>>>>
>>>>>> On 23/03/2021 18:21, Catalin Marinas wrote:
>>>>>>> Hi Suzuki?
>>>>>>>
>>>>>>> On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
>>>>>>>> tsb csync synchronizes the trace operation of instructions.
>>>>>>>> The instruction is a nop when FEAT_TRF is not implemented.
>>>>>>>>
>>>>>>>> Cc: Mathieu Poirier <[email protected]>
>>>>>>>> Cc: Mike Leach <[email protected]>
>>>>>>>> Cc: Catalin Marinas <[email protected]>
>>>>>>>> Cc: Will Deacon <[email protected]>
>>>>>>>> Signed-off-by: Suzuki K Poulose <[email protected]>
>>>>>>>
>>>>>>> How do you plan to merge these patches? If they go via the coresight
>>>>>>> tree:
>>>>>>>
>>>>>>
>>>>>> Ideally all of this should go via the CoreSight tree to have the
>>>>>> dependencies solved at one place. But there are some issues :
>>>>>>
>>>>>> If this makes to 5.13 queue for CoreSight,
>>>>>>
>>>>>> 1) CoreSight next is based on rc2 at the moment and we have fixes gone
>>>>>> into rc3 and later, which this series will depend on. (We could move
>>>>>> the next tree forward to a later rc to solve this).
>>>>>>
>>>>>> 2) There could be conflicts with the kvmarm tree for the KVM host
>>>>>> changes (That has dependency on the TRBE definitions patch).
>>>>>>
>>>>>> If it doesn't make to 5.13 queue, it would be good to have this patch,
>>>>>> the TRBE defintions and the KVM host patches queued for 5.13 (not sure
>>>>>> if this is acceptable) and we could rebase the CoreSight changes on 5.13
>>>>>> and push it to next release.
>>>>>>
>>>>>> I am open for other suggestions.
>>>>>>
>>>>>> Marc, Mathieu,
>>>>>>
>>>>>> Thoughts ?
>>>>>
>>>>> I was planning to take the first two patches in 5.12 as fixes (they
>>>>> are queued already, and would hopefully land in -rc5). If that doesn't
>>>>> fit with the plan, please let me know ASAP.
>>>>
>>>> Marc,
>>>>
>>>> I think it would be better to hold on pushing those patches until we
>>>> have a clarity on how things will go.
>>>
>>> OK. I thought there was a need for these patches to prevent guest
>>> access to the v8.4 self hosted tracing feature that went in 5.12
>>> though[1]... Did I get it wrong?
>>
>> Yes, that is correct. The guest could access the Trace Filter Control
>> register and fiddle with the host settings, without this patch.
>> e.g, it could disable tracing at EL0/EL1, without the host being
>> aware on nVHE host.
>
> OK, so we definitely do need these patches, don't we? Both? Just one?
> Please have a look at kvmarm/fixes and tell me what I must keep.

Both of them are fixes.

commit "KVM: arm64: Disable guest access to trace filter controls"
- This fixes guest fiddling with the trace filter control as described
above.

commit "KVM: arm64: Hide system instruction access to Trace registers"
- Fixes the Hypervisor to advertise what it doesn't support. i.e
stop advertising trace system instruction access to a guest.
Otherwise a guest which trusts the ID registers
(ID_AA64DFR0_EL1.TRACEVER == 1) can crash while trying to access the
trace register as we trap the accesses (CPTR_EL2.TTA == 1). On Linux,
the ETM drivers need a DT explicitly advertising the support. So,
this is not immediately impacted. And this fix goes a long way back
in the history, when the CPTR_EL2.TTA was added.

Now, the reason for asking you to hold on is the way this could create
conflicts in merging the rest of the series.

Suzuki

2021-03-24 17:21:13

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On Wed, Mar 24, 2021 at 05:06:58PM +0000, Suzuki K Poulose wrote:
> On 24/03/2021 16:30, Marc Zyngier wrote:
> > On Wed, 24 Mar 2021 16:25:12 +0000,
> > Suzuki K Poulose <[email protected]> wrote:
> > >
> > > On 24/03/2021 16:16, Marc Zyngier wrote:
> > > > On Wed, 24 Mar 2021 15:51:14 +0000,
> > > > Suzuki K Poulose <[email protected]> wrote:
> > > > >
> > > > > On 24/03/2021 13:49, Marc Zyngier wrote:
> > > > > > On Wed, 24 Mar 2021 09:39:13 +0000,
> > > > > > Suzuki K Poulose <[email protected]> wrote:
> > > > > > >
> > > > > > > On 23/03/2021 18:21, Catalin Marinas wrote:
> > > > > > > > Hi Suzuki?
> > > > > > > >
> > > > > > > > On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
> > > > > > > > > tsb csync synchronizes the trace operation of instructions.
> > > > > > > > > The instruction is a nop when FEAT_TRF is not implemented.
> > > > > > > > >
> > > > > > > > > Cc: Mathieu Poirier <[email protected]>
> > > > > > > > > Cc: Mike Leach <[email protected]>
> > > > > > > > > Cc: Catalin Marinas <[email protected]>
> > > > > > > > > Cc: Will Deacon <[email protected]>
> > > > > > > > > Signed-off-by: Suzuki K Poulose <[email protected]>
> > > > > > > >
> > > > > > > > How do you plan to merge these patches? If they go via the coresight
> > > > > > > > tree:
> > > > > > > >
> > > > > > >
> > > > > > > Ideally all of this should go via the CoreSight tree to have the
> > > > > > > dependencies solved at one place. But there are some issues :
> > > > > > >
> > > > > > > If this makes to 5.13 queue for CoreSight,
> > > > > > >
> > > > > > > 1) CoreSight next is based on rc2 at the moment and we have fixes gone
> > > > > > > into rc3 and later, which this series will depend on. (We could move
> > > > > > > the next tree forward to a later rc to solve this).
> > > > > > >
> > > > > > > 2) There could be conflicts with the kvmarm tree for the KVM host
> > > > > > > changes (That has dependency on the TRBE definitions patch).
> > > > > > >
> > > > > > > If it doesn't make to 5.13 queue, it would be good to have this patch,
> > > > > > > the TRBE defintions and the KVM host patches queued for 5.13 (not sure
> > > > > > > if this is acceptable) and we could rebase the CoreSight changes on 5.13
> > > > > > > and push it to next release.
> > > > > > >
> > > > > > > I am open for other suggestions.
> > > > > > >
> > > > > > > Marc, Mathieu,
> > > > > > >
> > > > > > > Thoughts ?
> > > > > >
> > > > > > I was planning to take the first two patches in 5.12 as fixes (they
> > > > > > are queued already, and would hopefully land in -rc5). If that doesn't
> > > > > > fit with the plan, please let me know ASAP.
> > > > >
> > > > > Marc,
> > > > >
> > > > > I think it would be better to hold on pushing those patches until we
> > > > > have a clarity on how things will go.
> > > >
> > > > OK. I thought there was a need for these patches to prevent guest
> > > > access to the v8.4 self hosted tracing feature that went in 5.12
> > > > though[1]... Did I get it wrong?
> > >
> > > Yes, that is correct. The guest could access the Trace Filter Control
> > > register and fiddle with the host settings, without this patch.
> > > e.g, it could disable tracing at EL0/EL1, without the host being
> > > aware on nVHE host.
> >
> > OK, so we definitely do need these patches, don't we? Both? Just one?
> > Please have a look at kvmarm/fixes and tell me what I must keep.
>
> Both of them are fixes.
>
> commit "KVM: arm64: Disable guest access to trace filter controls"
> - This fixes guest fiddling with the trace filter control as described
> above.
>
> commit "KVM: arm64: Hide system instruction access to Trace registers"
> - Fixes the Hypervisor to advertise what it doesn't support. i.e
> stop advertising trace system instruction access to a guest.
> Otherwise a guest which trusts the ID registers
> (ID_AA64DFR0_EL1.TRACEVER == 1) can crash while trying to access the
> trace register as we trap the accesses (CPTR_EL2.TTA == 1). On Linux,
> the ETM drivers need a DT explicitly advertising the support. So,
> this is not immediately impacted. And this fix goes a long way back
> in the history, when the CPTR_EL2.TTA was added.
>
> Now, the reason for asking you to hold on is the way this could create
> conflicts in merging the rest of the series.

The way we normally work around this is to either rebase your series on
top of -rc5 when the fixes go in or, if you want an earlier -rc base,
Marc can put them on a stable branch somewhere that you can use.

In the worst case you can merge the patches twice but that's rarely
needed.

--
Catalin

2021-03-24 17:42:56

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On Wed, 24 Mar 2021 17:19:36 +0000,
Catalin Marinas <[email protected]> wrote:
>
> On Wed, Mar 24, 2021 at 05:06:58PM +0000, Suzuki K Poulose wrote:
> > On 24/03/2021 16:30, Marc Zyngier wrote:
> > >
> > > OK, so we definitely do need these patches, don't we? Both? Just one?
> > > Please have a look at kvmarm/fixes and tell me what I must keep.
> >
> > Both of them are fixes.
> >
> > commit "KVM: arm64: Disable guest access to trace filter controls"
> > - This fixes guest fiddling with the trace filter control as described
> > above.
> >
> > commit "KVM: arm64: Hide system instruction access to Trace registers"
> > - Fixes the Hypervisor to advertise what it doesn't support. i.e
> > stop advertising trace system instruction access to a guest.
> > Otherwise a guest which trusts the ID registers
> > (ID_AA64DFR0_EL1.TRACEVER == 1) can crash while trying to access the
> > trace register as we trap the accesses (CPTR_EL2.TTA == 1). On Linux,
> > the ETM drivers need a DT explicitly advertising the support. So,
> > this is not immediately impacted. And this fix goes a long way back
> > in the history, when the CPTR_EL2.TTA was added.
> >
> > Now, the reason for asking you to hold on is the way this could create
> > conflicts in merging the rest of the series.
>
> The way we normally work around this is to either rebase your series on
> top of -rc5 when the fixes go in or, if you want an earlier -rc base,
> Marc can put them on a stable branch somewhere that you can use.

Here's what I've done:

- the two patches are now on a branch[1] based off -rc3 which I
officially declare stable. Feel free to rebase your series on top.

- the KVM fixes branch now embeds this branch (yes, I've rebased it --
we'll hopefully survive the outrage).

Thanks,

M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git/log/?h=trace-fixes-5.12

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

2021-03-25 02:30:37

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On 23/03/2021 18:21, Catalin Marinas wrote:
> Hi Suzuki?
>
> On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
>> tsb csync synchronizes the trace operation of instructions.
>> The instruction is a nop when FEAT_TRF is not implemented.
>>
>> Cc: Mathieu Poirier <[email protected]>
>> Cc: Mike Leach <[email protected]>
>> Cc: Catalin Marinas <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> Signed-off-by: Suzuki K Poulose <[email protected]>
>
> How do you plan to merge these patches? If they go via the coresight
> tree:
>

Ideally all of this should go via the CoreSight tree to have the
dependencies solved at one place. But there are some issues :

If this makes to 5.13 queue for CoreSight,

1) CoreSight next is based on rc2 at the moment and we have fixes gone
into rc3 and later, which this series will depend on. (We could move
the next tree forward to a later rc to solve this).

2) There could be conflicts with the kvmarm tree for the KVM host
changes (That has dependency on the TRBE definitions patch).

If it doesn't make to 5.13 queue, it would be good to have this patch,
the TRBE defintions and the KVM host patches queued for 5.13 (not sure
if this is acceptable) and we could rebase the CoreSight changes on 5.13
and push it to next release.

I am open for other suggestions.

Marc, Mathieu,

Thoughts ?

> Acked-by: Catalin Marinas <[email protected]>
>

Thanks
Suzuki

2021-03-25 03:15:40

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On Wed, 24 Mar 2021 15:51:14 +0000,
Suzuki K Poulose <[email protected]> wrote:
>
> On 24/03/2021 13:49, Marc Zyngier wrote:
> > On Wed, 24 Mar 2021 09:39:13 +0000,
> > Suzuki K Poulose <[email protected]> wrote:
> >>
> >> On 23/03/2021 18:21, Catalin Marinas wrote:
> >>> Hi Suzuki?
> >>>
> >>> On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
> >>>> tsb csync synchronizes the trace operation of instructions.
> >>>> The instruction is a nop when FEAT_TRF is not implemented.
> >>>>
> >>>> Cc: Mathieu Poirier <[email protected]>
> >>>> Cc: Mike Leach <[email protected]>
> >>>> Cc: Catalin Marinas <[email protected]>
> >>>> Cc: Will Deacon <[email protected]>
> >>>> Signed-off-by: Suzuki K Poulose <[email protected]>
> >>>
> >>> How do you plan to merge these patches? If they go via the coresight
> >>> tree:
> >>>
> >>
> >> Ideally all of this should go via the CoreSight tree to have the
> >> dependencies solved at one place. But there are some issues :
> >>
> >> If this makes to 5.13 queue for CoreSight,
> >>
> >> 1) CoreSight next is based on rc2 at the moment and we have fixes gone
> >> into rc3 and later, which this series will depend on. (We could move
> >> the next tree forward to a later rc to solve this).
> >>
> >> 2) There could be conflicts with the kvmarm tree for the KVM host
> >> changes (That has dependency on the TRBE definitions patch).
> >>
> >> If it doesn't make to 5.13 queue, it would be good to have this patch,
> >> the TRBE defintions and the KVM host patches queued for 5.13 (not sure
> >> if this is acceptable) and we could rebase the CoreSight changes on 5.13
> >> and push it to next release.
> >>
> >> I am open for other suggestions.
> >>
> >> Marc, Mathieu,
> >>
> >> Thoughts ?
> >
> > I was planning to take the first two patches in 5.12 as fixes (they
> > are queued already, and would hopefully land in -rc5). If that doesn't
> > fit with the plan, please let me know ASAP.
>
> Marc,
>
> I think it would be better to hold on pushing those patches until we
> have a clarity on how things will go.

OK. I thought there was a need for these patches to prevent guest
access to the v8.4 self hosted tracing feature that went in 5.12
though[1]... Did I get it wrong?

Thanks,

M.

[1] https://lore.kernel.org/r/[email protected]

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

2021-03-25 03:16:18

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On 24/03/2021 16:16, Marc Zyngier wrote:
> On Wed, 24 Mar 2021 15:51:14 +0000,
> Suzuki K Poulose <[email protected]> wrote:
>>
>> On 24/03/2021 13:49, Marc Zyngier wrote:
>>> On Wed, 24 Mar 2021 09:39:13 +0000,
>>> Suzuki K Poulose <[email protected]> wrote:
>>>>
>>>> On 23/03/2021 18:21, Catalin Marinas wrote:
>>>>> Hi Suzuki?
>>>>>
>>>>> On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
>>>>>> tsb csync synchronizes the trace operation of instructions.
>>>>>> The instruction is a nop when FEAT_TRF is not implemented.
>>>>>>
>>>>>> Cc: Mathieu Poirier <[email protected]>
>>>>>> Cc: Mike Leach <[email protected]>
>>>>>> Cc: Catalin Marinas <[email protected]>
>>>>>> Cc: Will Deacon <[email protected]>
>>>>>> Signed-off-by: Suzuki K Poulose <[email protected]>
>>>>>
>>>>> How do you plan to merge these patches? If they go via the coresight
>>>>> tree:
>>>>>
>>>>
>>>> Ideally all of this should go via the CoreSight tree to have the
>>>> dependencies solved at one place. But there are some issues :
>>>>
>>>> If this makes to 5.13 queue for CoreSight,
>>>>
>>>> 1) CoreSight next is based on rc2 at the moment and we have fixes gone
>>>> into rc3 and later, which this series will depend on. (We could move
>>>> the next tree forward to a later rc to solve this).
>>>>
>>>> 2) There could be conflicts with the kvmarm tree for the KVM host
>>>> changes (That has dependency on the TRBE definitions patch).
>>>>
>>>> If it doesn't make to 5.13 queue, it would be good to have this patch,
>>>> the TRBE defintions and the KVM host patches queued for 5.13 (not sure
>>>> if this is acceptable) and we could rebase the CoreSight changes on 5.13
>>>> and push it to next release.
>>>>
>>>> I am open for other suggestions.
>>>>
>>>> Marc, Mathieu,
>>>>
>>>> Thoughts ?
>>>
>>> I was planning to take the first two patches in 5.12 as fixes (they
>>> are queued already, and would hopefully land in -rc5). If that doesn't
>>> fit with the plan, please let me know ASAP.
>>
>> Marc,
>>
>> I think it would be better to hold on pushing those patches until we
>> have a clarity on how things will go.
>
> OK. I thought there was a need for these patches to prevent guest
> access to the v8.4 self hosted tracing feature that went in 5.12
> though[1]... Did I get it wrong?

Yes, that is correct. The guest could access the Trace Filter Control
register and fiddle with the host settings, without this patch.
e.g, it could disable tracing at EL0/EL1, without the host being
aware on nVHE host.

Cheers
Suzuki

2021-03-25 03:16:38

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On Wed, 24 Mar 2021 16:25:12 +0000,
Suzuki K Poulose <[email protected]> wrote:
>
> On 24/03/2021 16:16, Marc Zyngier wrote:
> > On Wed, 24 Mar 2021 15:51:14 +0000,
> > Suzuki K Poulose <[email protected]> wrote:
> >>
> >> On 24/03/2021 13:49, Marc Zyngier wrote:
> >>> On Wed, 24 Mar 2021 09:39:13 +0000,
> >>> Suzuki K Poulose <[email protected]> wrote:
> >>>>
> >>>> On 23/03/2021 18:21, Catalin Marinas wrote:
> >>>>> Hi Suzuki?
> >>>>>
> >>>>> On Tue, Mar 23, 2021 at 12:06:33PM +0000, Suzuki K Poulose wrote:
> >>>>>> tsb csync synchronizes the trace operation of instructions.
> >>>>>> The instruction is a nop when FEAT_TRF is not implemented.
> >>>>>>
> >>>>>> Cc: Mathieu Poirier <[email protected]>
> >>>>>> Cc: Mike Leach <[email protected]>
> >>>>>> Cc: Catalin Marinas <[email protected]>
> >>>>>> Cc: Will Deacon <[email protected]>
> >>>>>> Signed-off-by: Suzuki K Poulose <[email protected]>
> >>>>>
> >>>>> How do you plan to merge these patches? If they go via the coresight
> >>>>> tree:
> >>>>>
> >>>>
> >>>> Ideally all of this should go via the CoreSight tree to have the
> >>>> dependencies solved at one place. But there are some issues :
> >>>>
> >>>> If this makes to 5.13 queue for CoreSight,
> >>>>
> >>>> 1) CoreSight next is based on rc2 at the moment and we have fixes gone
> >>>> into rc3 and later, which this series will depend on. (We could move
> >>>> the next tree forward to a later rc to solve this).
> >>>>
> >>>> 2) There could be conflicts with the kvmarm tree for the KVM host
> >>>> changes (That has dependency on the TRBE definitions patch).
> >>>>
> >>>> If it doesn't make to 5.13 queue, it would be good to have this patch,
> >>>> the TRBE defintions and the KVM host patches queued for 5.13 (not sure
> >>>> if this is acceptable) and we could rebase the CoreSight changes on 5.13
> >>>> and push it to next release.
> >>>>
> >>>> I am open for other suggestions.
> >>>>
> >>>> Marc, Mathieu,
> >>>>
> >>>> Thoughts ?
> >>>
> >>> I was planning to take the first two patches in 5.12 as fixes (they
> >>> are queued already, and would hopefully land in -rc5). If that doesn't
> >>> fit with the plan, please let me know ASAP.
> >>
> >> Marc,
> >>
> >> I think it would be better to hold on pushing those patches until we
> >> have a clarity on how things will go.
> >
> > OK. I thought there was a need for these patches to prevent guest
> > access to the v8.4 self hosted tracing feature that went in 5.12
> > though[1]... Did I get it wrong?
>
> Yes, that is correct. The guest could access the Trace Filter Control
> register and fiddle with the host settings, without this patch.
> e.g, it could disable tracing at EL0/EL1, without the host being
> aware on nVHE host.

OK, so we definitely do need these patches, don't we? Both? Just one?
Please have a look at kvmarm/fixes and tell me what I must keep.

Thanks,

M.

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

2021-03-26 16:33:37

by Mathieu Poirier

[permalink] [raw]
Subject: Re: [PATCH v5 05/19] arm64: Add support for trace synchronization barrier

On Wed, Mar 24, 2021 at 05:40:39PM +0000, Marc Zyngier wrote:
> On Wed, 24 Mar 2021 17:19:36 +0000,
> Catalin Marinas <[email protected]> wrote:
> >
> > On Wed, Mar 24, 2021 at 05:06:58PM +0000, Suzuki K Poulose wrote:
> > > On 24/03/2021 16:30, Marc Zyngier wrote:
> > > >
> > > > OK, so we definitely do need these patches, don't we? Both? Just one?
> > > > Please have a look at kvmarm/fixes and tell me what I must keep.
> > >
> > > Both of them are fixes.
> > >
> > > commit "KVM: arm64: Disable guest access to trace filter controls"
> > > - This fixes guest fiddling with the trace filter control as described
> > > above.
> > >
> > > commit "KVM: arm64: Hide system instruction access to Trace registers"
> > > - Fixes the Hypervisor to advertise what it doesn't support. i.e
> > > stop advertising trace system instruction access to a guest.
> > > Otherwise a guest which trusts the ID registers
> > > (ID_AA64DFR0_EL1.TRACEVER == 1) can crash while trying to access the
> > > trace register as we trap the accesses (CPTR_EL2.TTA == 1). On Linux,
> > > the ETM drivers need a DT explicitly advertising the support. So,
> > > this is not immediately impacted. And this fix goes a long way back
> > > in the history, when the CPTR_EL2.TTA was added.
> > >
> > > Now, the reason for asking you to hold on is the way this could create
> > > conflicts in merging the rest of the series.
> >
> > The way we normally work around this is to either rebase your series on
> > top of -rc5 when the fixes go in or, if you want an earlier -rc base,
> > Marc can put them on a stable branch somewhere that you can use.
>
> Here's what I've done:
>
> - the two patches are now on a branch[1] based off -rc3 which I
> officially declare stable. Feel free to rebase your series on top.
>
> - the KVM fixes branch now embeds this branch (yes, I've rebased it --
> we'll hopefully survive the outrage).

We don't have a choice to rebase. I will rebased CS next on [1] and
apply this set on top of it. Hopefully the KVM fixes will have made it to GKH's
char-misc tree by the time I send him the patches for the next merge window.
Otherwise we'll have to merge patches twice as Catalin mentioned. We can deal
with that if/when we get there.

Thanks,
Mathieu

>
> Thanks,
>
> M.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git/log/?h=trace-fixes-5.12
>
> --
> Without deviation from the norm, progress is not possible.