Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On 30/11/2022 08:13, Atish Kumar Patra wrote: > In Linux DT binding or dt schema repo ? I am a bit confused now as we > talked about both above :). I asked about a dt-binding and not a schema change, so the former ;)
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On Tue, Nov 29, 2022 at 3:54 PM wrote:
>
> +CC Rob, which I probably should've done earlier, so
> context all preserved
>
> On 29/11/2022 09:42, Conor Dooley wrote:
> > On 29/11/2022 09:27, Atish Kumar Patra wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> >> content is safe
> >>
> >> On Mon, Nov 28, 2022 at 11:32 PM wrote:
> >>>
> >>> On 29/11/2022 07:08, Andrew Jones wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> the content is safe
>
> On Mon, Nov 28, 2022 at 09:10:03PM +, conor.doo...@microchip.com
> wrote:
> > On 28/11/2022 20:41, Atish Kumar Patra wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >> the content is safe
> >>
> >> On Mon, Nov 28, 2022 at 12:38 PM wrote:
> >>>
> >>> On 28/11/2022 20:16, Atish Kumar Patra wrote:
> On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
> wrote:
> >
> > On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> >> Qemu virt machine can support few cache events and cycle/instret
> >> counters.
> >> It also supports counter overflow for these events.
> >>
> >> Add a DT node so that OpenSBI/Linux kernel is aware of the virt
> >> machine
> >> capabilities. There are some dummy nodes added for testing as well.
> >
> > Hey Atish!
> >
> > I was fiddling with dumping the virt machine dtb again today to
> > check
> > some dt-binding changes I was making for the isa string would play
> > nicely with the virt machine & I noticed that this patch has
> > introduced
> > a new validation failure:
> >
> > ./build/qemu-system-riscv64 -nographic -machine
> > virt,dumpdtb=qemu.dtb
> >
> > dt-validate -p
> > ../linux/Documentation/devicetree/bindings/processed-schema.json
> > qemu.dtb
> > /home/conor/stuff/qemu/qemu.dtb: soc: pmu:
> > {'riscv,event-to-mhpmcounters': [[1, 1, 524281, 2, 2, 524284,
> > 65561, 65561, 524280, 65563, 65563, 524280, 65569, 65569, 524280,
> > 0, 0, 0, 0, 0]], 'compatible': ['riscv,pmu']} should not be valid
> > under {'type': 'object'}
> > From schema:
> > /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
> >
> > I assume this is the aforementioned "dummy" node & you have no
> > intention
> > of creating a binding for this?
> >
>
> It is a dummy node from Linux kernel perspective. OpenSbi use this
> node to figure out the hpmcounter mappings.
> >>>
> >>> Aye, but should it not have a binding anyway, since they're not
> >>> meant to be linux specific?
> >>>
> >> It is documented in OpenSBI.
> >> https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> >>
> >> Are you suggesting that any non-Linux specific DT nodes should be part
> >> of Linux DT binding as well ?
> >
> > I thought the point was that they were *not* meant to be linux specific,
> > just happening to be housed there.
> >
>
> I'm not sure if there's an official policy on where DT nodes should be
> specified, but it looks like Samuel's opinion is that they should live
> in the Linux kernel, whether they're used there or not [1].
>
> [1] http://lists.infradead.org/pipermail/opensbi/2022-October/003522.html
> >>>
> >>> Yah, that was also my understanding. See also U-Boot moving to unify
> >>> their custom bindings into the linux repo:
> >>> https://lore.kernel.org/linux-devicetree/20220930001410.2802843-1-...@chromium.org/
> >>>
> >>
> >> This adds the U-Boot specific DT properties to the dts schema itself,
> >> not Linux kernel DT bindings.
> >
> > Yeah, sorry. I muddled things up a little there. My point was that they
> > are trying to get to a stage where dt-validate and those tools work for
> > them too. I'm not sure were I said "linux repo" rather than "dt-schema
> > repo" when I double checked the file paths in the link before pasting it
> > to make sure it was the dt-schema one.. I blame it being early.
> >
> >> I am not opposed to adding PMU DT bindings to Linux but there should
> >> be a clear policy on this.
> >> What about OpenSBI domain DT bindings ?
> >> If every other DT based open source project starts adding their DT
> >> binding to the Linux kernel, that may go downhill pretty soon.
>
> Rob, perhaps you can be a source of clarity here :) My early morning
> muddling didn't help things.
>
>
> > Maybe I am misunderstanding, but I had thought the goal was to get to
> > user-independent bindings. Rob and Krzysztof certainly labour the point
> > that the bindings should not reflect how one operating system's drivers
> >
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
+CC Rob, which I probably should've done earlier, so
context all preserved
On 29/11/2022 09:42, Conor Dooley wrote:
> On 29/11/2022 09:27, Atish Kumar Patra wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>>
>> On Mon, Nov 28, 2022 at 11:32 PM wrote:
>>>
>>> On 29/11/2022 07:08, Andrew Jones wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the
content is safe
On Mon, Nov 28, 2022 at 09:10:03PM +, conor.doo...@microchip.com wrote:
> On 28/11/2022 20:41, Atish Kumar Patra wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> On Mon, Nov 28, 2022 at 12:38 PM wrote:
>>>
>>> On 28/11/2022 20:16, Atish Kumar Patra wrote:
On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
wrote:
>
> On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
>> Qemu virt machine can support few cache events and cycle/instret
>> counters.
>> It also supports counter overflow for these events.
>>
>> Add a DT node so that OpenSBI/Linux kernel is aware of the virt
>> machine
>> capabilities. There are some dummy nodes added for testing as well.
>
> Hey Atish!
>
> I was fiddling with dumping the virt machine dtb again today to check
> some dt-binding changes I was making for the isa string would play
> nicely with the virt machine & I noticed that this patch has
> introduced
> a new validation failure:
>
> ./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
>
> dt-validate -p
> ../linux/Documentation/devicetree/bindings/processed-schema.json
> qemu.dtb
> /home/conor/stuff/qemu/qemu.dtb: soc: pmu:
> {'riscv,event-to-mhpmcounters': [[1, 1, 524281, 2, 2, 524284, 65561,
> 65561, 524280, 65563, 65563, 524280, 65569, 65569, 524280, 0, 0, 0,
> 0, 0]], 'compatible': ['riscv,pmu']} should not be valid under
> {'type': 'object'}
> From schema:
> /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
>
> I assume this is the aforementioned "dummy" node & you have no
> intention
> of creating a binding for this?
>
It is a dummy node from Linux kernel perspective. OpenSbi use this
node to figure out the hpmcounter mappings.
>>>
>>> Aye, but should it not have a binding anyway, since they're not
>>> meant to be linux specific?
>>>
>> It is documented in OpenSBI.
>> https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
>>
>> Are you suggesting that any non-Linux specific DT nodes should be part
>> of Linux DT binding as well ?
>
> I thought the point was that they were *not* meant to be linux specific,
> just happening to be housed there.
>
I'm not sure if there's an official policy on where DT nodes should be
specified, but it looks like Samuel's opinion is that they should live
in the Linux kernel, whether they're used there or not [1].
[1] http://lists.infradead.org/pipermail/opensbi/2022-October/003522.html
>>>
>>> Yah, that was also my understanding. See also U-Boot moving to unify
>>> their custom bindings into the linux repo:
>>> https://lore.kernel.org/linux-devicetree/20220930001410.2802843-1-...@chromium.org/
>>>
>>
>> This adds the U-Boot specific DT properties to the dts schema itself,
>> not Linux kernel DT bindings.
>
> Yeah, sorry. I muddled things up a little there. My point was that they
> are trying to get to a stage where dt-validate and those tools work for
> them too. I'm not sure were I said "linux repo" rather than "dt-schema
> repo" when I double checked the file paths in the link before pasting it
> to make sure it was the dt-schema one.. I blame it being early.
>
>> I am not opposed to adding PMU DT bindings to Linux but there should
>> be a clear policy on this.
>> What about OpenSBI domain DT bindings ?
>> If every other DT based open source project starts adding their DT
>> binding to the Linux kernel, that may go downhill pretty soon.
Rob, perhaps you can be a source of clarity here :) My early morning
muddling didn't help things.
> Maybe I am misunderstanding, but I had thought the goal was to get to
> user-independent bindings. Rob and Krzysztof certainly labour the point
> that the bindings should not reflect how one operating system's drivers
> would like to see them & u-boot or FreeBSD using a property is grounds
> for it not being removed from the bindings in the linux tree.
>
> I'll go and actually ask Rob.
I did go & ask Rob, to which he said "I'll apply it even if no driver."
Do you want to whip up a binding, or shall I?
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On 29/11/2022 09:27, Atish Kumar Patra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Mon, Nov 28, 2022 at 11:32 PM wrote:
>>
>> On 29/11/2022 07:08, Andrew Jones wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>>> content is safe
>>>
>>> On Mon, Nov 28, 2022 at 09:10:03PM +, conor.doo...@microchip.com wrote:
On 28/11/2022 20:41, Atish Kumar Patra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> the content is safe
>
> On Mon, Nov 28, 2022 at 12:38 PM wrote:
>>
>> On 28/11/2022 20:16, Atish Kumar Patra wrote:
>>> On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
>>> wrote:
On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> Qemu virt machine can support few cache events and cycle/instret
> counters.
> It also supports counter overflow for these events.
>
> Add a DT node so that OpenSBI/Linux kernel is aware of the virt
> machine
> capabilities. There are some dummy nodes added for testing as well.
Hey Atish!
I was fiddling with dumping the virt machine dtb again today to check
some dt-binding changes I was making for the isa string would play
nicely with the virt machine & I noticed that this patch has introduced
a new validation failure:
./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
dt-validate -p
../linux/Documentation/devicetree/bindings/processed-schema.json
qemu.dtb
/home/conor/stuff/qemu/qemu.dtb: soc: pmu:
{'riscv,event-to-mhpmcounters': [[1, 1, 524281, 2, 2, 524284, 65561,
65561, 524280, 65563, 65563, 524280, 65569, 65569, 524280, 0, 0, 0, 0,
0]], 'compatible': ['riscv,pmu']} should not be valid under {'type':
'object'}
From schema:
/home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
I assume this is the aforementioned "dummy" node & you have no
intention
of creating a binding for this?
>>>
>>> It is a dummy node from Linux kernel perspective. OpenSbi use this
>>> node to figure out the hpmcounter mappings.
>>
>> Aye, but should it not have a binding anyway, since they're not
>> meant to be linux specific?
>>
> It is documented in OpenSBI.
> https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
>
> Are you suggesting that any non-Linux specific DT nodes should be part
> of Linux DT binding as well ?
I thought the point was that they were *not* meant to be linux specific,
just happening to be housed there.
>>>
>>> I'm not sure if there's an official policy on where DT nodes should be
>>> specified, but it looks like Samuel's opinion is that they should live
>>> in the Linux kernel, whether they're used there or not [1].
>>>
>>> [1] http://lists.infradead.org/pipermail/opensbi/2022-October/003522.html
>>
>> Yah, that was also my understanding. See also U-Boot moving to unify
>> their custom bindings into the linux repo:
>> https://lore.kernel.org/linux-devicetree/20220930001410.2802843-1-...@chromium.org/
>>
>
> This adds the U-Boot specific DT properties to the dts schema itself,
> not Linux kernel DT bindings.
Yeah, sorry. I muddled things up a little there. My point was that they
are trying to get to a stage where dt-validate and those tools work for
them too. I'm not sure were I said "linux repo" rather than "dt-schema
repo" when I double checked the file paths in the link before pasting it
to make sure it was the dt-schema one.. I blame it being early.
> I am not opposed to adding PMU DT bindings to Linux but there should
> be a clear policy on this.
> What about OpenSBI domain DT bindings ?
> If every other DT based open source project starts adding their DT
> binding to the Linux kernel, that may go downhill pretty soon.
Maybe I am misunderstanding, but I had thought the goal was to get to
user-independent bindings. Rob and Krzysztof certainly labour the point
that the bindings should not reflect how one operating system's drivers
would like to see them & u-boot or FreeBSD using a property is grounds
for it not being removed from the bindings in the linux tree.
I'll go and actually ask Rob.
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On Mon, Nov 28, 2022 at 11:32 PM wrote:
>
> On 29/11/2022 07:08, Andrew Jones wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > On Mon, Nov 28, 2022 at 09:10:03PM +, conor.doo...@microchip.com wrote:
> >> On 28/11/2022 20:41, Atish Kumar Patra wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >>> the content is safe
> >>>
> >>> On Mon, Nov 28, 2022 at 12:38 PM wrote:
>
> On 28/11/2022 20:16, Atish Kumar Patra wrote:
> > On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
> > wrote:
> >>
> >> On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> >>> Qemu virt machine can support few cache events and cycle/instret
> >>> counters.
> >>> It also supports counter overflow for these events.
> >>>
> >>> Add a DT node so that OpenSBI/Linux kernel is aware of the virt
> >>> machine
> >>> capabilities. There are some dummy nodes added for testing as well.
> >>
> >> Hey Atish!
> >>
> >> I was fiddling with dumping the virt machine dtb again today to check
> >> some dt-binding changes I was making for the isa string would play
> >> nicely with the virt machine & I noticed that this patch has introduced
> >> a new validation failure:
> >>
> >> ./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
> >>
> >> dt-validate -p
> >> ../linux/Documentation/devicetree/bindings/processed-schema.json
> >> qemu.dtb
> >> /home/conor/stuff/qemu/qemu.dtb: soc: pmu:
> >> {'riscv,event-to-mhpmcounters': [[1, 1, 524281, 2, 2, 524284, 65561,
> >> 65561, 524280, 65563, 65563, 524280, 65569, 65569, 524280, 0, 0, 0, 0,
> >> 0]], 'compatible': ['riscv,pmu']} should not be valid under {'type':
> >> 'object'}
> >> From schema:
> >> /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
> >>
> >> I assume this is the aforementioned "dummy" node & you have no
> >> intention
> >> of creating a binding for this?
> >>
> >
> > It is a dummy node from Linux kernel perspective. OpenSbi use this
> > node to figure out the hpmcounter mappings.
>
> Aye, but should it not have a binding anyway, since they're not
> meant to be linux specific?
>
> >>> It is documented in OpenSBI.
> >>> https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> >>>
> >>> Are you suggesting that any non-Linux specific DT nodes should be part
> >>> of Linux DT binding as well ?
> >>
> >> I thought the point was that they were *not* meant to be linux specific,
> >> just happening to be housed there.
> >>
> >
> > I'm not sure if there's an official policy on where DT nodes should be
> > specified, but it looks like Samuel's opinion is that they should live
> > in the Linux kernel, whether they're used there or not [1].
> >
> > [1] http://lists.infradead.org/pipermail/opensbi/2022-October/003522.html
>
> Yah, that was also my understanding. See also U-Boot moving to unify
> their custom bindings into the linux repo:
> https://lore.kernel.org/linux-devicetree/20220930001410.2802843-1-...@chromium.org/
>
This adds the U-Boot specific DT properties to the dts schema itself,
not Linux kernel DT bindings.
I am not opposed to adding PMU DT bindings to Linux but there should
be a clear policy on this.
What about OpenSBI domain DT bindings ?
If every other DT based open source project starts adding their DT
binding to the Linux kernel, that may go downhill pretty soon.
>
>
>
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On 29/11/2022 07:08, Andrew Jones wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Mon, Nov 28, 2022 at 09:10:03PM +, conor.doo...@microchip.com wrote:
>> On 28/11/2022 20:41, Atish Kumar Patra wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>>> content is safe
>>>
>>> On Mon, Nov 28, 2022 at 12:38 PM wrote:
On 28/11/2022 20:16, Atish Kumar Patra wrote:
> On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
> wrote:
>>
>> On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
>>> Qemu virt machine can support few cache events and cycle/instret
>>> counters.
>>> It also supports counter overflow for these events.
>>>
>>> Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
>>> capabilities. There are some dummy nodes added for testing as well.
>>
>> Hey Atish!
>>
>> I was fiddling with dumping the virt machine dtb again today to check
>> some dt-binding changes I was making for the isa string would play
>> nicely with the virt machine & I noticed that this patch has introduced
>> a new validation failure:
>>
>> ./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
>>
>> dt-validate -p
>> ../linux/Documentation/devicetree/bindings/processed-schema.json qemu.dtb
>> /home/conor/stuff/qemu/qemu.dtb: soc: pmu:
>> {'riscv,event-to-mhpmcounters': [[1, 1, 524281, 2, 2, 524284, 65561,
>> 65561, 524280, 65563, 65563, 524280, 65569, 65569, 524280, 0, 0, 0, 0,
>> 0]], 'compatible': ['riscv,pmu']} should not be valid under {'type':
>> 'object'}
>> From schema:
>> /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
>>
>> I assume this is the aforementioned "dummy" node & you have no intention
>> of creating a binding for this?
>>
>
> It is a dummy node from Linux kernel perspective. OpenSbi use this
> node to figure out the hpmcounter mappings.
Aye, but should it not have a binding anyway, since they're not
meant to be linux specific?
>>> It is documented in OpenSBI.
>>> https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
>>>
>>> Are you suggesting that any non-Linux specific DT nodes should be part
>>> of Linux DT binding as well ?
>>
>> I thought the point was that they were *not* meant to be linux specific,
>> just happening to be housed there.
>>
>
> I'm not sure if there's an official policy on where DT nodes should be
> specified, but it looks like Samuel's opinion is that they should live
> in the Linux kernel, whether they're used there or not [1].
>
> [1] http://lists.infradead.org/pipermail/opensbi/2022-October/003522.html
Yah, that was also my understanding. See also U-Boot moving to unify
their custom bindings into the linux repo:
https://lore.kernel.org/linux-devicetree/20220930001410.2802843-1-...@chromium.org/
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On Mon, Nov 28, 2022 at 09:10:03PM +, conor.doo...@microchip.com wrote:
> On 28/11/2022 20:41, Atish Kumar Patra wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > On Mon, Nov 28, 2022 at 12:38 PM wrote:
> >>
> >> On 28/11/2022 20:16, Atish Kumar Patra wrote:
> >>> On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
> >>> wrote:
>
> On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> > Qemu virt machine can support few cache events and cycle/instret
> > counters.
> > It also supports counter overflow for these events.
> >
> > Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
> > capabilities. There are some dummy nodes added for testing as well.
>
> Hey Atish!
>
> I was fiddling with dumping the virt machine dtb again today to check
> some dt-binding changes I was making for the isa string would play
> nicely with the virt machine & I noticed that this patch has introduced
> a new validation failure:
>
> ./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
>
> dt-validate -p
> ../linux/Documentation/devicetree/bindings/processed-schema.json qemu.dtb
> /home/conor/stuff/qemu/qemu.dtb: soc: pmu:
> {'riscv,event-to-mhpmcounters': [[1, 1, 524281, 2, 2, 524284, 65561,
> 65561, 524280, 65563, 65563, 524280, 65569, 65569, 524280, 0, 0, 0, 0,
> 0]], 'compatible': ['riscv,pmu']} should not be valid under {'type':
> 'object'}
> From schema:
> /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
>
> I assume this is the aforementioned "dummy" node & you have no intention
> of creating a binding for this?
>
> >>>
> >>> It is a dummy node from Linux kernel perspective. OpenSbi use this
> >>> node to figure out the hpmcounter mappings.
> >>
> >> Aye, but should it not have a binding anyway, since they're not
> >> meant to be linux specific?
> >>
> > It is documented in OpenSBI.
> > https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> >
> > Are you suggesting that any non-Linux specific DT nodes should be part
> > of Linux DT binding as well ?
>
> I thought the point was that they were *not* meant to be linux specific,
> just happening to be housed there.
>
I'm not sure if there's an official policy on where DT nodes should be
specified, but it looks like Samuel's opinion is that they should live
in the Linux kernel, whether they're used there or not [1].
[1] http://lists.infradead.org/pipermail/opensbi/2022-October/003522.html
Thanks,
drew
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On 28/11/2022 20:41, Atish Kumar Patra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Mon, Nov 28, 2022 at 12:38 PM wrote:
>>
>> On 28/11/2022 20:16, Atish Kumar Patra wrote:
>>> On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
>>> wrote:
On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> Qemu virt machine can support few cache events and cycle/instret counters.
> It also supports counter overflow for these events.
>
> Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
> capabilities. There are some dummy nodes added for testing as well.
Hey Atish!
I was fiddling with dumping the virt machine dtb again today to check
some dt-binding changes I was making for the isa string would play
nicely with the virt machine & I noticed that this patch has introduced
a new validation failure:
./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
dt-validate -p
../linux/Documentation/devicetree/bindings/processed-schema.json qemu.dtb
/home/conor/stuff/qemu/qemu.dtb: soc: pmu: {'riscv,event-to-mhpmcounters':
[[1, 1, 524281, 2, 2, 524284, 65561, 65561, 524280, 65563, 65563, 524280,
65569, 65569, 524280, 0, 0, 0, 0, 0]], 'compatible': ['riscv,pmu']} should
not be valid under {'type': 'object'}
From schema:
/home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
I assume this is the aforementioned "dummy" node & you have no intention
of creating a binding for this?
>>>
>>> It is a dummy node from Linux kernel perspective. OpenSbi use this
>>> node to figure out the hpmcounter mappings.
>>
>> Aye, but should it not have a binding anyway, since they're not
>> meant to be linux specific?
>>
> It is documented in OpenSBI.
> https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
>
> Are you suggesting that any non-Linux specific DT nodes should be part
> of Linux DT binding as well ?
I thought the point was that they were *not* meant to be linux specific,
just happening to be housed there.
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On Mon, Nov 28, 2022 at 12:38 PM wrote:
>
> On 28/11/2022 20:16, Atish Kumar Patra wrote:
> > On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
> > wrote:
> >>
> >> On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> >>> Qemu virt machine can support few cache events and cycle/instret counters.
> >>> It also supports counter overflow for these events.
> >>>
> >>> Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
> >>> capabilities. There are some dummy nodes added for testing as well.
> >>
> >> Hey Atish!
> >>
> >> I was fiddling with dumping the virt machine dtb again today to check
> >> some dt-binding changes I was making for the isa string would play
> >> nicely with the virt machine & I noticed that this patch has introduced
> >> a new validation failure:
> >>
> >> ./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
> >>
> >> dt-validate -p
> >> ../linux/Documentation/devicetree/bindings/processed-schema.json qemu.dtb
> >> /home/conor/stuff/qemu/qemu.dtb: soc: pmu: {'riscv,event-to-mhpmcounters':
> >> [[1, 1, 524281, 2, 2, 524284, 65561, 65561, 524280, 65563, 65563, 524280,
> >> 65569, 65569, 524280, 0, 0, 0, 0, 0]], 'compatible': ['riscv,pmu']} should
> >> not be valid under {'type': 'object'}
> >> From schema:
> >> /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
> >>
> >> I assume this is the aforementioned "dummy" node & you have no intention
> >> of creating a binding for this?
> >>
> >
> > It is a dummy node from Linux kernel perspective. OpenSbi use this
> > node to figure out the hpmcounter mappings.
>
> Aye, but should it not have a binding anyway, since they're not
> meant to be linux specific?
>
It is documented in OpenSBI.
https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
Are you suggesting that any non-Linux specific DT nodes should be part
of Linux DT binding as well ?
> >>> Acked-by: Alistair Francis
> >>> Signed-off-by: Atish Patra
> >>> Signed-off-by: Atish Patra
> >>> ---
> >>> hw/riscv/virt.c| 16 +
> >>> target/riscv/pmu.c | 57 ++
> >>> target/riscv/pmu.h | 1 +
> >>> 3 files changed, 74 insertions(+)
> >>>
> >>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> >>> index ff8c0df5cd47..befa9d2c26ac 100644
> >>> --- a/hw/riscv/virt.c
> >>> +++ b/hw/riscv/virt.c
> >>> @@ -30,6 +30,7 @@
> >>> #include "hw/char/serial.h"
> >>> #include "target/riscv/cpu.h"
> >>> #include "hw/core/sysbus-fdt.h"
> >>> +#include "target/riscv/pmu.h"
> >>> #include "hw/riscv/riscv_hart.h"
> >>> #include "hw/riscv/virt.h"
> >>> #include "hw/riscv/boot.h"
> >>> @@ -708,6 +709,20 @@ static void create_fdt_socket_aplic(RISCVVirtState
> >>> *s,
> >>> aplic_phandles[socket] = aplic_s_phandle;
> >>> }
> >>>
> >>> +static void create_fdt_pmu(RISCVVirtState *s)
> >>> +{
> >>> +char *pmu_name;
> >>> +MachineState *mc = MACHINE(s);
> >>> +RISCVCPU hart = s->soc[0].harts[0];
> >>> +
> >>> +pmu_name = g_strdup_printf("/soc/pmu");
> >>> +qemu_fdt_add_subnode(mc->fdt, pmu_name);
> >>> +qemu_fdt_setprop_string(mc->fdt, pmu_name, "compatible",
> >>> "riscv,pmu");
> >>> +riscv_pmu_generate_fdt_node(mc->fdt, hart.cfg.pmu_num, pmu_name);
> >>> +
> >>> +g_free(pmu_name);
> >>> +}
> >>> +
> >>> static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry
> >>> *memmap,
> >>> bool is_32_bit, uint32_t *phandle,
> >>> uint32_t *irq_mmio_phandle,
> >>> @@ -1036,6 +1051,7 @@ static void create_fdt(RISCVVirtState *s, const
> >>> MemMapEntry *memmap,
> >>>
> >>> create_fdt_flash(s, memmap);
> >>> create_fdt_fw_cfg(s, memmap);
> >>> +create_fdt_pmu(s);
> >>>
> >>> update_bootargs:
> >>> if (cmdline && *cmdline) {
> >>> diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
> >>> index a5f504e53c88..b8e56d2b7b8e 100644
> >>> --- a/target/riscv/pmu.c
> >>> +++ b/target/riscv/pmu.c
> >>> @@ -20,11 +20,68 @@
> >>> #include "cpu.h"
> >>> #include "pmu.h"
> >>> #include "sysemu/cpu-timers.h"
> >>> +#include "sysemu/device_tree.h"
> >>>
> >>> #define RISCV_TIMEBASE_FREQ 10 /* 1Ghz */
> >>> #define MAKE_32BIT_MASK(shift, length) \
> >>> (((uint32_t)(~0UL) >> (32 - (length))) << (shift))
> >>>
> >>> +/*
> >>> + * To keep it simple, any event can be mapped to any programmable
> >>> counters in
> >>> + * QEMU. The generic cycle & instruction count events can also be
> >>> monitored
> >>> + * using programmable counters. In that case, mcycle & minstret must
> >>> continue
> >>> + * to provide the correct value as well. Heterogeneous PMU per hart is
> >>> not
> >>> + * supported yet. Thus, number of counters are same across all harts.
> >>> + */
> >>> +void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
> >>> +{
> >>> +uint32_t fdt_event_ctr_map[20] = {};
> >>> +uint32_t
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On 28/11/2022 20:16, Atish Kumar Patra wrote:
> On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley
> wrote:
>>
>> On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
>>> Qemu virt machine can support few cache events and cycle/instret counters.
>>> It also supports counter overflow for these events.
>>>
>>> Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
>>> capabilities. There are some dummy nodes added for testing as well.
>>
>> Hey Atish!
>>
>> I was fiddling with dumping the virt machine dtb again today to check
>> some dt-binding changes I was making for the isa string would play
>> nicely with the virt machine & I noticed that this patch has introduced
>> a new validation failure:
>>
>> ./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
>>
>> dt-validate -p
>> ../linux/Documentation/devicetree/bindings/processed-schema.json qemu.dtb
>> /home/conor/stuff/qemu/qemu.dtb: soc: pmu: {'riscv,event-to-mhpmcounters':
>> [[1, 1, 524281, 2, 2, 524284, 65561, 65561, 524280, 65563, 65563, 524280,
>> 65569, 65569, 524280, 0, 0, 0, 0, 0]], 'compatible': ['riscv,pmu']} should
>> not be valid under {'type': 'object'}
>> From schema:
>> /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
>>
>> I assume this is the aforementioned "dummy" node & you have no intention
>> of creating a binding for this?
>>
>
> It is a dummy node from Linux kernel perspective. OpenSbi use this
> node to figure out the hpmcounter mappings.
Aye, but should it not have a binding anyway, since they're not
meant to be linux specific?
>>> Acked-by: Alistair Francis
>>> Signed-off-by: Atish Patra
>>> Signed-off-by: Atish Patra
>>> ---
>>> hw/riscv/virt.c| 16 +
>>> target/riscv/pmu.c | 57 ++
>>> target/riscv/pmu.h | 1 +
>>> 3 files changed, 74 insertions(+)
>>>
>>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>>> index ff8c0df5cd47..befa9d2c26ac 100644
>>> --- a/hw/riscv/virt.c
>>> +++ b/hw/riscv/virt.c
>>> @@ -30,6 +30,7 @@
>>> #include "hw/char/serial.h"
>>> #include "target/riscv/cpu.h"
>>> #include "hw/core/sysbus-fdt.h"
>>> +#include "target/riscv/pmu.h"
>>> #include "hw/riscv/riscv_hart.h"
>>> #include "hw/riscv/virt.h"
>>> #include "hw/riscv/boot.h"
>>> @@ -708,6 +709,20 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
>>> aplic_phandles[socket] = aplic_s_phandle;
>>> }
>>>
>>> +static void create_fdt_pmu(RISCVVirtState *s)
>>> +{
>>> +char *pmu_name;
>>> +MachineState *mc = MACHINE(s);
>>> +RISCVCPU hart = s->soc[0].harts[0];
>>> +
>>> +pmu_name = g_strdup_printf("/soc/pmu");
>>> +qemu_fdt_add_subnode(mc->fdt, pmu_name);
>>> +qemu_fdt_setprop_string(mc->fdt, pmu_name, "compatible", "riscv,pmu");
>>> +riscv_pmu_generate_fdt_node(mc->fdt, hart.cfg.pmu_num, pmu_name);
>>> +
>>> +g_free(pmu_name);
>>> +}
>>> +
>>> static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry
>>> *memmap,
>>> bool is_32_bit, uint32_t *phandle,
>>> uint32_t *irq_mmio_phandle,
>>> @@ -1036,6 +1051,7 @@ static void create_fdt(RISCVVirtState *s, const
>>> MemMapEntry *memmap,
>>>
>>> create_fdt_flash(s, memmap);
>>> create_fdt_fw_cfg(s, memmap);
>>> +create_fdt_pmu(s);
>>>
>>> update_bootargs:
>>> if (cmdline && *cmdline) {
>>> diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
>>> index a5f504e53c88..b8e56d2b7b8e 100644
>>> --- a/target/riscv/pmu.c
>>> +++ b/target/riscv/pmu.c
>>> @@ -20,11 +20,68 @@
>>> #include "cpu.h"
>>> #include "pmu.h"
>>> #include "sysemu/cpu-timers.h"
>>> +#include "sysemu/device_tree.h"
>>>
>>> #define RISCV_TIMEBASE_FREQ 10 /* 1Ghz */
>>> #define MAKE_32BIT_MASK(shift, length) \
>>> (((uint32_t)(~0UL) >> (32 - (length))) << (shift))
>>>
>>> +/*
>>> + * To keep it simple, any event can be mapped to any programmable counters
>>> in
>>> + * QEMU. The generic cycle & instruction count events can also be monitored
>>> + * using programmable counters. In that case, mcycle & minstret must
>>> continue
>>> + * to provide the correct value as well. Heterogeneous PMU per hart is not
>>> + * supported yet. Thus, number of counters are same across all harts.
>>> + */
>>> +void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
>>> +{
>>> +uint32_t fdt_event_ctr_map[20] = {};
>>> +uint32_t cmask;
>>> +
>>> +/* All the programmable counters can map to any event */
>>> +cmask = MAKE_32BIT_MASK(3, num_ctrs);
>>> +
>>> + /*
>>> +* The event encoding is specified in the SBI specification
>>> +* Event idx is a 20bits wide number encoded as follows:
>>> +* event_idx[19:16] = type
>>> +* event_idx[15:0] = code
>>> +* The code field in cache events are encoded as follows:
>>> +* event_idx.code[15:3] = cache_id
>>> +* event_idx.code[2:1] = op_id
>>> +*
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On Thu, Nov 24, 2022 at 5:17 AM Conor Dooley wrote:
>
> On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> > Qemu virt machine can support few cache events and cycle/instret counters.
> > It also supports counter overflow for these events.
> >
> > Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
> > capabilities. There are some dummy nodes added for testing as well.
>
> Hey Atish!
>
> I was fiddling with dumping the virt machine dtb again today to check
> some dt-binding changes I was making for the isa string would play
> nicely with the virt machine & I noticed that this patch has introduced
> a new validation failure:
>
> ./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
>
> dt-validate -p
> ../linux/Documentation/devicetree/bindings/processed-schema.json qemu.dtb
> /home/conor/stuff/qemu/qemu.dtb: soc: pmu: {'riscv,event-to-mhpmcounters':
> [[1, 1, 524281, 2, 2, 524284, 65561, 65561, 524280, 65563, 65563, 524280,
> 65569, 65569, 524280, 0, 0, 0, 0, 0]], 'compatible': ['riscv,pmu']} should
> not be valid under {'type': 'object'}
> From schema:
> /home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
>
> I assume this is the aforementioned "dummy" node & you have no intention
> of creating a binding for this?
>
It is a dummy node from Linux kernel perspective. OpenSbi use this
node to figure out the hpmcounter mappings.
> Thanks,
> Conor.
>
> > Acked-by: Alistair Francis
> > Signed-off-by: Atish Patra
> > Signed-off-by: Atish Patra
> > ---
> > hw/riscv/virt.c| 16 +
> > target/riscv/pmu.c | 57 ++
> > target/riscv/pmu.h | 1 +
> > 3 files changed, 74 insertions(+)
> >
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index ff8c0df5cd47..befa9d2c26ac 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -30,6 +30,7 @@
> > #include "hw/char/serial.h"
> > #include "target/riscv/cpu.h"
> > #include "hw/core/sysbus-fdt.h"
> > +#include "target/riscv/pmu.h"
> > #include "hw/riscv/riscv_hart.h"
> > #include "hw/riscv/virt.h"
> > #include "hw/riscv/boot.h"
> > @@ -708,6 +709,20 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
> > aplic_phandles[socket] = aplic_s_phandle;
> > }
> >
> > +static void create_fdt_pmu(RISCVVirtState *s)
> > +{
> > +char *pmu_name;
> > +MachineState *mc = MACHINE(s);
> > +RISCVCPU hart = s->soc[0].harts[0];
> > +
> > +pmu_name = g_strdup_printf("/soc/pmu");
> > +qemu_fdt_add_subnode(mc->fdt, pmu_name);
> > +qemu_fdt_setprop_string(mc->fdt, pmu_name, "compatible", "riscv,pmu");
> > +riscv_pmu_generate_fdt_node(mc->fdt, hart.cfg.pmu_num, pmu_name);
> > +
> > +g_free(pmu_name);
> > +}
> > +
> > static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry
> > *memmap,
> > bool is_32_bit, uint32_t *phandle,
> > uint32_t *irq_mmio_phandle,
> > @@ -1036,6 +1051,7 @@ static void create_fdt(RISCVVirtState *s, const
> > MemMapEntry *memmap,
> >
> > create_fdt_flash(s, memmap);
> > create_fdt_fw_cfg(s, memmap);
> > +create_fdt_pmu(s);
> >
> > update_bootargs:
> > if (cmdline && *cmdline) {
> > diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
> > index a5f504e53c88..b8e56d2b7b8e 100644
> > --- a/target/riscv/pmu.c
> > +++ b/target/riscv/pmu.c
> > @@ -20,11 +20,68 @@
> > #include "cpu.h"
> > #include "pmu.h"
> > #include "sysemu/cpu-timers.h"
> > +#include "sysemu/device_tree.h"
> >
> > #define RISCV_TIMEBASE_FREQ 10 /* 1Ghz */
> > #define MAKE_32BIT_MASK(shift, length) \
> > (((uint32_t)(~0UL) >> (32 - (length))) << (shift))
> >
> > +/*
> > + * To keep it simple, any event can be mapped to any programmable counters
> > in
> > + * QEMU. The generic cycle & instruction count events can also be monitored
> > + * using programmable counters. In that case, mcycle & minstret must
> > continue
> > + * to provide the correct value as well. Heterogeneous PMU per hart is not
> > + * supported yet. Thus, number of counters are same across all harts.
> > + */
> > +void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
> > +{
> > +uint32_t fdt_event_ctr_map[20] = {};
> > +uint32_t cmask;
> > +
> > +/* All the programmable counters can map to any event */
> > +cmask = MAKE_32BIT_MASK(3, num_ctrs);
> > +
> > + /*
> > +* The event encoding is specified in the SBI specification
> > +* Event idx is a 20bits wide number encoded as follows:
> > +* event_idx[19:16] = type
> > +* event_idx[15:0] = code
> > +* The code field in cache events are encoded as follows:
> > +* event_idx.code[15:3] = cache_id
> > +* event_idx.code[2:1] = op_id
> > +* event_idx.code[0:0] = result_id
> > +*/
> > +
> > + /* SBI_PMU_HW_CPU_CYCLES: 0x01 : type(0x00) */
> > + fdt_event_ctr_map[0] = cpu_to_be32(0x0001);
>
Re: [PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
On Wed, Aug 24, 2022 at 03:17:00PM -0700, Atish Patra wrote:
> Qemu virt machine can support few cache events and cycle/instret counters.
> It also supports counter overflow for these events.
>
> Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
> capabilities. There are some dummy nodes added for testing as well.
Hey Atish!
I was fiddling with dumping the virt machine dtb again today to check
some dt-binding changes I was making for the isa string would play
nicely with the virt machine & I noticed that this patch has introduced
a new validation failure:
./build/qemu-system-riscv64 -nographic -machine virt,dumpdtb=qemu.dtb
dt-validate -p ../linux/Documentation/devicetree/bindings/processed-schema.json
qemu.dtb
/home/conor/stuff/qemu/qemu.dtb: soc: pmu: {'riscv,event-to-mhpmcounters': [[1,
1, 524281, 2, 2, 524284, 65561, 65561, 524280, 65563, 65563, 524280, 65569,
65569, 524280, 0, 0, 0, 0, 0]], 'compatible': ['riscv,pmu']} should not be
valid under {'type': 'object'}
From schema:
/home/conor/.local/lib/python3.10/site-packages/dtschema/schemas/simple-bus.yaml
I assume this is the aforementioned "dummy" node & you have no intention
of creating a binding for this?
Thanks,
Conor.
> Acked-by: Alistair Francis
> Signed-off-by: Atish Patra
> Signed-off-by: Atish Patra
> ---
> hw/riscv/virt.c| 16 +
> target/riscv/pmu.c | 57 ++
> target/riscv/pmu.h | 1 +
> 3 files changed, 74 insertions(+)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index ff8c0df5cd47..befa9d2c26ac 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -30,6 +30,7 @@
> #include "hw/char/serial.h"
> #include "target/riscv/cpu.h"
> #include "hw/core/sysbus-fdt.h"
> +#include "target/riscv/pmu.h"
> #include "hw/riscv/riscv_hart.h"
> #include "hw/riscv/virt.h"
> #include "hw/riscv/boot.h"
> @@ -708,6 +709,20 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
> aplic_phandles[socket] = aplic_s_phandle;
> }
>
> +static void create_fdt_pmu(RISCVVirtState *s)
> +{
> +char *pmu_name;
> +MachineState *mc = MACHINE(s);
> +RISCVCPU hart = s->soc[0].harts[0];
> +
> +pmu_name = g_strdup_printf("/soc/pmu");
> +qemu_fdt_add_subnode(mc->fdt, pmu_name);
> +qemu_fdt_setprop_string(mc->fdt, pmu_name, "compatible", "riscv,pmu");
> +riscv_pmu_generate_fdt_node(mc->fdt, hart.cfg.pmu_num, pmu_name);
> +
> +g_free(pmu_name);
> +}
> +
> static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
> bool is_32_bit, uint32_t *phandle,
> uint32_t *irq_mmio_phandle,
> @@ -1036,6 +1051,7 @@ static void create_fdt(RISCVVirtState *s, const
> MemMapEntry *memmap,
>
> create_fdt_flash(s, memmap);
> create_fdt_fw_cfg(s, memmap);
> +create_fdt_pmu(s);
>
> update_bootargs:
> if (cmdline && *cmdline) {
> diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
> index a5f504e53c88..b8e56d2b7b8e 100644
> --- a/target/riscv/pmu.c
> +++ b/target/riscv/pmu.c
> @@ -20,11 +20,68 @@
> #include "cpu.h"
> #include "pmu.h"
> #include "sysemu/cpu-timers.h"
> +#include "sysemu/device_tree.h"
>
> #define RISCV_TIMEBASE_FREQ 10 /* 1Ghz */
> #define MAKE_32BIT_MASK(shift, length) \
> (((uint32_t)(~0UL) >> (32 - (length))) << (shift))
>
> +/*
> + * To keep it simple, any event can be mapped to any programmable counters in
> + * QEMU. The generic cycle & instruction count events can also be monitored
> + * using programmable counters. In that case, mcycle & minstret must continue
> + * to provide the correct value as well. Heterogeneous PMU per hart is not
> + * supported yet. Thus, number of counters are same across all harts.
> + */
> +void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
> +{
> +uint32_t fdt_event_ctr_map[20] = {};
> +uint32_t cmask;
> +
> +/* All the programmable counters can map to any event */
> +cmask = MAKE_32BIT_MASK(3, num_ctrs);
> +
> + /*
> +* The event encoding is specified in the SBI specification
> +* Event idx is a 20bits wide number encoded as follows:
> +* event_idx[19:16] = type
> +* event_idx[15:0] = code
> +* The code field in cache events are encoded as follows:
> +* event_idx.code[15:3] = cache_id
> +* event_idx.code[2:1] = op_id
> +* event_idx.code[0:0] = result_id
> +*/
> +
> + /* SBI_PMU_HW_CPU_CYCLES: 0x01 : type(0x00) */
> + fdt_event_ctr_map[0] = cpu_to_be32(0x0001);
> + fdt_event_ctr_map[1] = cpu_to_be32(0x0001);
> + fdt_event_ctr_map[2] = cpu_to_be32(cmask | 1 << 0);
> +
> + /* SBI_PMU_HW_INSTRUCTIONS: 0x02 : type(0x00) */
> + fdt_event_ctr_map[3] = cpu_to_be32(0x0002);
> + fdt_event_ctr_map[4] = cpu_to_be32(0x0002);
> + fdt_event_ctr_map[5] = cpu_to_be32(cmask | 1 << 2);
> +
> + /* SBI_PMU_HW_CACHE_DTLB : 0x03 READ : 0x00 MISS : 0x00 type(0x01)
[PATCH v14 4/5] hw/riscv: virt: Add PMU DT node to the device tree
Qemu virt machine can support few cache events and cycle/instret counters.
It also supports counter overflow for these events.
Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
capabilities. There are some dummy nodes added for testing as well.
Acked-by: Alistair Francis
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
hw/riscv/virt.c| 16 +
target/riscv/pmu.c | 57 ++
target/riscv/pmu.h | 1 +
3 files changed, 74 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index ff8c0df5cd47..befa9d2c26ac 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -30,6 +30,7 @@
#include "hw/char/serial.h"
#include "target/riscv/cpu.h"
#include "hw/core/sysbus-fdt.h"
+#include "target/riscv/pmu.h"
#include "hw/riscv/riscv_hart.h"
#include "hw/riscv/virt.h"
#include "hw/riscv/boot.h"
@@ -708,6 +709,20 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
aplic_phandles[socket] = aplic_s_phandle;
}
+static void create_fdt_pmu(RISCVVirtState *s)
+{
+char *pmu_name;
+MachineState *mc = MACHINE(s);
+RISCVCPU hart = s->soc[0].harts[0];
+
+pmu_name = g_strdup_printf("/soc/pmu");
+qemu_fdt_add_subnode(mc->fdt, pmu_name);
+qemu_fdt_setprop_string(mc->fdt, pmu_name, "compatible", "riscv,pmu");
+riscv_pmu_generate_fdt_node(mc->fdt, hart.cfg.pmu_num, pmu_name);
+
+g_free(pmu_name);
+}
+
static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
bool is_32_bit, uint32_t *phandle,
uint32_t *irq_mmio_phandle,
@@ -1036,6 +1051,7 @@ static void create_fdt(RISCVVirtState *s, const
MemMapEntry *memmap,
create_fdt_flash(s, memmap);
create_fdt_fw_cfg(s, memmap);
+create_fdt_pmu(s);
update_bootargs:
if (cmdline && *cmdline) {
diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index a5f504e53c88..b8e56d2b7b8e 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -20,11 +20,68 @@
#include "cpu.h"
#include "pmu.h"
#include "sysemu/cpu-timers.h"
+#include "sysemu/device_tree.h"
#define RISCV_TIMEBASE_FREQ 10 /* 1Ghz */
#define MAKE_32BIT_MASK(shift, length) \
(((uint32_t)(~0UL) >> (32 - (length))) << (shift))
+/*
+ * To keep it simple, any event can be mapped to any programmable counters in
+ * QEMU. The generic cycle & instruction count events can also be monitored
+ * using programmable counters. In that case, mcycle & minstret must continue
+ * to provide the correct value as well. Heterogeneous PMU per hart is not
+ * supported yet. Thus, number of counters are same across all harts.
+ */
+void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
+{
+uint32_t fdt_event_ctr_map[20] = {};
+uint32_t cmask;
+
+/* All the programmable counters can map to any event */
+cmask = MAKE_32BIT_MASK(3, num_ctrs);
+
+ /*
+* The event encoding is specified in the SBI specification
+* Event idx is a 20bits wide number encoded as follows:
+* event_idx[19:16] = type
+* event_idx[15:0] = code
+* The code field in cache events are encoded as follows:
+* event_idx.code[15:3] = cache_id
+* event_idx.code[2:1] = op_id
+* event_idx.code[0:0] = result_id
+*/
+
+ /* SBI_PMU_HW_CPU_CYCLES: 0x01 : type(0x00) */
+ fdt_event_ctr_map[0] = cpu_to_be32(0x0001);
+ fdt_event_ctr_map[1] = cpu_to_be32(0x0001);
+ fdt_event_ctr_map[2] = cpu_to_be32(cmask | 1 << 0);
+
+ /* SBI_PMU_HW_INSTRUCTIONS: 0x02 : type(0x00) */
+ fdt_event_ctr_map[3] = cpu_to_be32(0x0002);
+ fdt_event_ctr_map[4] = cpu_to_be32(0x0002);
+ fdt_event_ctr_map[5] = cpu_to_be32(cmask | 1 << 2);
+
+ /* SBI_PMU_HW_CACHE_DTLB : 0x03 READ : 0x00 MISS : 0x00 type(0x01) */
+ fdt_event_ctr_map[6] = cpu_to_be32(0x00010019);
+ fdt_event_ctr_map[7] = cpu_to_be32(0x00010019);
+ fdt_event_ctr_map[8] = cpu_to_be32(cmask);
+
+ /* SBI_PMU_HW_CACHE_DTLB : 0x03 WRITE : 0x01 MISS : 0x00 type(0x01) */
+ fdt_event_ctr_map[9] = cpu_to_be32(0x0001001B);
+ fdt_event_ctr_map[10] = cpu_to_be32(0x0001001B);
+ fdt_event_ctr_map[11] = cpu_to_be32(cmask);
+
+ /* SBI_PMU_HW_CACHE_ITLB : 0x04 READ : 0x00 MISS : 0x00 type(0x01) */
+ fdt_event_ctr_map[12] = cpu_to_be32(0x00010021);
+ fdt_event_ctr_map[13] = cpu_to_be32(0x00010021);
+ fdt_event_ctr_map[14] = cpu_to_be32(cmask);
+
+ /* This a OpenSBI specific DT property documented in OpenSBI docs */
+ qemu_fdt_setprop(fdt, pmu_name, "riscv,event-to-mhpmcounters",
+fdt_event_ctr_map, sizeof(fdt_event_ctr_map));
+}
+
static bool riscv_pmu_counter_valid(RISCVCPU *cpu, uint32_t ctr_idx)
{
if (ctr_idx < 3 || ctr_idx >= RV_MAX_MHPMCOUNTERS ||
diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
index 036653627f78..3004ce37b636 100644
--- a/target/riscv/pmu.h
+++ b/target/riscv/pmu.h
@@ -31,5 +31,6 @@ int riscv_pmu_init(RISCVCPU *cpu, int num_counters);
