[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v13 09/10] xen/riscv: introduce ANDN_INSN
On 25.06.2024 20:26, Oleksii wrote: > On Tue, 2024-06-25 at 16:49 +0200, Jan Beulich wrote: >> On 25.06.2024 15:51, Oleksii Kurochko wrote: >>> --- a/xen/arch/riscv/include/asm/cmpxchg.h >>> +++ b/xen/arch/riscv/include/asm/cmpxchg.h >>> @@ -18,6 +18,20 @@ >>> : "r" (new) \ >>> : "memory" ); >>> >>> +/* >>> + * Binutils < 2.37 doesn't understand ANDN. If the toolchain is >>> too >>> +ld, form >> >> Same question: Why's 2.37 suddenly of interest? > Andrew has (or had) an older version of binutils and started to face > the issues mentioned in this patch. As a result, some > changes were suggested. > >> Plus, why would age of the >> tool chain matter? > As it is mentioned in the comment binutils < 2.37 doesn't understand > andn instruction. But that's not the point. If the tool chain is too old, our logic to detect that should arrange for __riscv_zbb to not be set. That's all that needed to cover gas not understanding the insn. The rest here isn't about the capabilities of the tool chain: Either we make Zbb a requirement (at which point .insn can be used to encode ANDN), or we don't (at which point the replacement code you have comes into play). >> What you care about is whether you're permitted to use >> the extension at runtime. > At the moment we can't check that at runtime, w/o having an exception, > ( there is some option to check when dts parsing will be available in > Xen ). I will add the check when dts parsing functionality will be > available. Right now the best what we can do it is just mentioned that > as requirement in docs. > >> Otherwise you could again ... >> >> Also something went wrong with line wrapping here. >> >>> + * it of a NOT+AND pair >>> + */ >>> +#ifdef __riscv_zbb >>> +#define ANDN_INSN(rd, rs1, rs2) \ >>> + "andn " rd ", " rs1 ", " rs2 "\n" >>> +#else >>> +#define ANDN_INSN(rd, rs1, rs2) \ >>> + "not " rd ", " rs2 "\n" \ >>> + "and " rd ", " rs1 ", " rd "\n" >> >> ... resort to .insn. > Hmm, good point, it could be an issue. > > > If this patch is still needed (Andrew, have you updated your > toolchain?), then it should use .insn instead of andn. However, using > .insn requires encoding rd, rs1, and rs2 through asm ("some_reg") (?), > which seems overly complicated. Why? You don't want to use the raw form of .insn (which, as per the other sub-thread on this series, is available from gas 2.38 only anyway), but the one permitting operands to be spelled out (.insn r ...), along the lines of what I suggested for "pause". Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |