Re: [PATCH] target/riscv: Fix PMU node property for virt machine

[Conor Dooley]

On Thu, Apr 27, 2023 at 01:28:18PM +0800, Yu-Chien Peter Lin wrote:
> Hi Conor,
> 
> Thank you for your prompt response.
> 
> On Fri, Apr 21, 2023 at 06:59:40PM +0100, Conor Dooley wrote:
> > On Fri, Apr 21, 2023 at 09:14:37PM +0800, Yu Chien Peter Lin wrote:
> > > The length of fdt_event_ctr_map[20] will add 5 dummy cells in
> > > "riscv,event-to-mhpmcounters" property, so directly initialize
> > > the array without an explicit size.
> > > 
> > > This patch also fixes the typo of PMU cache operation result ID
> > > of MISS (0x1) in the comments, and renames event idx 0x10021 to
> > > RISCV_PMU_EVENT_CACHE_ITLB_READ_MISS.
> > > 
> > > Signed-off-by: Yu Chien Peter Lin <peterlin@andestech.com>
> > > ---
> > > 
> > >   $ ./build/qemu-system-riscv64 -M virt,dumpdtb=/tmp/virt.dtb -cpu 
> > > rv64,sscofpmf=on && dtc /tmp/virt.dtb | grep mhpmcounters
> > >   [...]
> > >     riscv,event-to-mhpmcounters = <0x01 0x01 0x7fff9 
> > >                                    0x02 0x02 0x7fffc
> > >                                    0x10019 0x10019 0x7fff8
> > >                                    0x1001b 0x1001b 0x7fff8
> > >                                    0x10021 0x10021 0x7fff8
> > >                dummy cells --->    0x00 0x00 0x00 0x00 0x00>;
> > > 
> > > This won't break the OpenSBI, but will cause it to incorrectly increment
> > > num_hw_events [1] to 6, and DT validation failure in kernel [2].
> > > 
> > >   $ dt-validate -p 
> > > Documentation/devicetree/bindings/processed-schema.json virt.dtb
> > >   [...]
> > >   virt.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'}
> > 
> > I would note that this warning here does not go away with this patch ^^
> > It's still on my todo list, unless you want to fix it!
> 
> I don't fully understand the warning raised by simple-bus.yaml
> is it reasonable to move pmu out of soc node?

If it has no reg properties, it should not be on the soc bus.
I previously made similar changes to other nodes, see commit
ae29379998 ("hw/riscv: virt: fix syscon subnode paths"), as I think it
is indeed the same change here.

Cheers,
Conor.

signature.asc
Description: PGP signature