Hi, On Mon, Dec 12, 2022 at 09:05:02AM +0000, Marc Zyngier wrote: > Alex, > > On Sun, 11 Dec 2022 11:40:39 +0000, > Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote: > > > > A simple "hey, you're wrong here, the PMU extensions do not follow the > > principles of the ID scheme for fields in ID registers" would have > > sufficed. > > This is what I did, and saved you the hassle of looking it up. The comment was about how you went about it, not about proving someone wrong. As expressive as it might be, I don't think that calling someone's suggestion "ludicrous" from the position of authority associated with being a maintainer is constructive; and can also be interpreted as a personal attack (you used **your** suggestion, not **this** suggestion). I didn't interpret it that way, just saying that it can be. > > > Guess you never made a silly mistake ever, right? > > It's not so much about making a silly mistake. I do that all the time. > But it is about the way you state these things, and the weight that > your reviews carry. You're a trusted reviewer, with a lot of > experience, and posting with an @arm.com address: what you say in a > public forum sticks. When you assert that the author is wrong, they > will take it at face value. This is how I stated things: "Hm... in the Arm ARM it says that counters are 64-bit if PMUv3p5 is implemented. But it doesn't say anywhere that versions newer than p5 are required to implement PMUv3p5." -> patently false, easily provable with the Arm ARM and by logic (as you did). My entire argument was based on this, so once this has been proven false, I would say that the rest of my argument falls apart. "For example, for PMUv3p7, it says that the feature is mandatory in Arm8.7 implementations. **My interpretation** of that is that it is not forbidden for an implementer to cherry-pick this version on older versions of the architecture where PMUv3p5 is not implemented." -> emphasis on the "my interpretation"; also easy to prove false because PMUv3p5+ is required to implement PMUv3p5, as per the architecture. "**Maybe** the check should be pmu.version == ID_DFR0_PMU_V3_8_5, to match the counter definitions in the architecture?" -> emphasis on the "maybe", and the question mark at the end. My intention wasn't to dictate something, my intention was to have a conversation about the patch, with the mindset that I might be wrong. What made you get the idea that I was asserting that the author is wrong? Where by "asserting the author is wrong" I understand framing my comment in such a way as to leave no room for further discussions. Or did you mean something else by that? Or, to put it another way, what about the way I stated things could have been done better (other than not being wrong, obviously)? > > > Otherwise, good job encouraging people to help review KVM/arm64 patches ;) > > What is the worse: no review? or a review that spreads confusion? > Think about it. I'm all for being nice, but I will call bullshit when That wasn't about calling people out on their mistakes. I was saying that the way you "call bullshit", as you put it, might be a put off for some people. Call me naive, but I like to think that not everyone that comments on a patch does it because they have to. > I see it asserted by people with a certain level of authority. > > And I've long made up my mind about the state of the KVM/arm64 review > process -- reviews rarely come from people who have volunteered to do > so, but instead from those who have either a vested interest in it, or > an ulterior motive. Hey ho... I genuinely don't know what to make of this. I can't even tell if it's directed at me or not. Thanks, Alex
