* [PATCH] Arm32: MSR to SPSR needs qualification
@ 2021-06-11 7:55 Jan Beulich
2021-06-11 8:00 ` Julien Grall
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2021-06-11 7:55 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, Julien Grall
The Arm ARM's description of MSR doesn't even allow for plain "SPSR"
here, and while gas accepts this, it takes it to mean SPSR_cf. Yet
surely all of SPSR wants updating on this path, not just the lowest and
highest 8 bits.
Fixes: dfcffb128be4 ("xen/arm32: SPSR_hyp/SPSR")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -395,7 +395,7 @@ return_to_hypervisor:
ldr r11, [sp, #UREGS_pc]
msr ELR_hyp, r11
ldr r11, [sp, #UREGS_cpsr]
- msr SPSR, r11
+ msr SPSR_cxsf, r11
#ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR
/*
* Hardening branch predictor may require to setup a different
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Arm32: MSR to SPSR needs qualification 2021-06-11 7:55 [PATCH] Arm32: MSR to SPSR needs qualification Jan Beulich @ 2021-06-11 8:00 ` Julien Grall 2021-06-11 9:16 ` Jan Beulich 0 siblings, 1 reply; 6+ messages in thread From: Julien Grall @ 2021-06-11 8:00 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 923 bytes --] On Fri, 11 Jun 2021, 08:55 Jan Beulich, <jbeulich@suse.com> wrote: > The Arm ARM's description of MSR doesn't even allow for plain "SPSR" > here, and while gas accepts this, it takes it to mean SPSR_cf. Yet > surely all of SPSR wants updating on this path, not just the lowest and > highest 8 bits. > Can you provide a reference to the Arm Arm? This would help to navigate through the 8000 pages. Cheers, > Fixes: dfcffb128be4 ("xen/arm32: SPSR_hyp/SPSR") > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/arm/arm32/entry.S > +++ b/xen/arch/arm/arm32/entry.S > @@ -395,7 +395,7 @@ return_to_hypervisor: > ldr r11, [sp, #UREGS_pc] > msr ELR_hyp, r11 > ldr r11, [sp, #UREGS_cpsr] > - msr SPSR, r11 > + msr SPSR_cxsf, r11 > #ifdef CONFIG_ARM32_HARDEN_BRANCH_PREDICTOR > /* > * Hardening branch predictor may require to setup a different > > [-- Attachment #2: Type: text/html, Size: 1709 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Arm32: MSR to SPSR needs qualification 2021-06-11 8:00 ` Julien Grall @ 2021-06-11 9:16 ` Jan Beulich 2021-06-11 10:41 ` Julien Grall 0 siblings, 1 reply; 6+ messages in thread From: Jan Beulich @ 2021-06-11 9:16 UTC (permalink / raw) To: Julien Grall; +Cc: xen-devel, Stefano Stabellini On 11.06.2021 10:00, Julien Grall wrote: > On Fri, 11 Jun 2021, 08:55 Jan Beulich, <jbeulich@suse.com> wrote: > >> The Arm ARM's description of MSR doesn't even allow for plain "SPSR" >> here, and while gas accepts this, it takes it to mean SPSR_cf. Yet >> surely all of SPSR wants updating on this path, not just the lowest and >> highest 8 bits. >> > > Can you provide a reference to the Arm Arm? This would help to navigate > through the 8000 pages. Referencing the instruction page would be enough, I thought (as even I, not being an Arm person, have no difficulty locating it). If it isn't, how is a canonical doc ref supposed to look like on Arm? On x86, we avoid recording document versions, section numbers, or even page numbers in code comments or commit messages (which isn't to say we have none of these, but we try to avoid new ones to appear), as these tend to change with every new version of the doc. Therefore, to me, the offending commit's "ARM DDI 0487D.b page G8-5993" doesn't look like something I wanted to clone from. But if you tell me otherwise, then well - so be it. Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Arm32: MSR to SPSR needs qualification 2021-06-11 9:16 ` Jan Beulich @ 2021-06-11 10:41 ` Julien Grall 2021-06-11 13:02 ` Jan Beulich 0 siblings, 1 reply; 6+ messages in thread From: Julien Grall @ 2021-06-11 10:41 UTC (permalink / raw) To: Jan Beulich; +Cc: Julien Grall, xen-devel, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 1547 bytes --] On Fri, 11 Jun 2021, 11:16 Jan Beulich, <jbeulich@suse.com> wrote: > On 11.06.2021 10:00, Julien Grall wrote: > > On Fri, 11 Jun 2021, 08:55 Jan Beulich, <jbeulich@suse.com> wrote: > > > >> The Arm ARM's description of MSR doesn't even allow for plain "SPSR" > >> here, and while gas accepts this, it takes it to mean SPSR_cf. Yet > >> surely all of SPSR wants updating on this path, not just the lowest and > >> highest 8 bits. > >> > > > > Can you provide a reference to the Arm Arm? This would help to navigate > > through the 8000 pages. > > Referencing the instruction page would be enough, I thought (as > even I, not being an Arm person, have no difficulty locating it). > If it isn't, how is a canonical doc ref supposed to look like on > Arm? On x86, we avoid recording document versions, section > numbers, or even page numbers in code comments or commit messages > (which isn't to say we have none of these, but we try to avoid > new ones to appear), as these tend to change with every new > version of the doc. Therefore, to me, the offending commit's "ARM > DDI 0487D.b page G8-5993" doesn't look like something I wanted to > clone from. But if you tell me otherwise, then well - so be it. The Arm website provides a link for nearly every revision on the specs. As the wording can change between version, it is useful to know which spec the understanding is based from. Note that for Arm32 we should quote the Armv7 spec and not the Armv8 one because we only follow the former (there are a few small differences). > Jan > > > [-- Attachment #2: Type: text/html, Size: 2547 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Arm32: MSR to SPSR needs qualification 2021-06-11 10:41 ` Julien Grall @ 2021-06-11 13:02 ` Jan Beulich 2021-06-11 13:22 ` Julien Grall 0 siblings, 1 reply; 6+ messages in thread From: Jan Beulich @ 2021-06-11 13:02 UTC (permalink / raw) To: Julien Grall; +Cc: Julien Grall, xen-devel, Stefano Stabellini On 11.06.2021 12:41, Julien Grall wrote: > On Fri, 11 Jun 2021, 11:16 Jan Beulich, <jbeulich@suse.com> wrote: > >> On 11.06.2021 10:00, Julien Grall wrote: >>> On Fri, 11 Jun 2021, 08:55 Jan Beulich, <jbeulich@suse.com> wrote: >>> >>>> The Arm ARM's description of MSR doesn't even allow for plain "SPSR" >>>> here, and while gas accepts this, it takes it to mean SPSR_cf. Yet >>>> surely all of SPSR wants updating on this path, not just the lowest and >>>> highest 8 bits. >>>> >>> >>> Can you provide a reference to the Arm Arm? This would help to navigate >>> through the 8000 pages. >> >> Referencing the instruction page would be enough, I thought (as >> even I, not being an Arm person, have no difficulty locating it). >> If it isn't, how is a canonical doc ref supposed to look like on >> Arm? On x86, we avoid recording document versions, section >> numbers, or even page numbers in code comments or commit messages >> (which isn't to say we have none of these, but we try to avoid >> new ones to appear), as these tend to change with every new >> version of the doc. Therefore, to me, the offending commit's "ARM >> DDI 0487D.b page G8-5993" doesn't look like something I wanted to >> clone from. But if you tell me otherwise, then well - so be it. > > > The Arm website provides a link for nearly every revision on the specs. As > the wording can change between version, it is useful to know which spec the > understanding is based from. > > Note that for Arm32 we should quote the Armv7 spec and not the Armv8 one > because we only follow the former (there are a few small differences). Thanks for having me dig out an up-to-date Armv7 spec. I find this puzzling in particular because you didn't care to have the earlier commit provide a v7 doc ref. Initially I did intentionally use (a newer version of) the doc that was pointed at there (which I also think is better structured than the v7 one). Jan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Arm32: MSR to SPSR needs qualification 2021-06-11 13:02 ` Jan Beulich @ 2021-06-11 13:22 ` Julien Grall 0 siblings, 0 replies; 6+ messages in thread From: Julien Grall @ 2021-06-11 13:22 UTC (permalink / raw) To: Jan Beulich; +Cc: Julien Grall, xen-devel, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 2509 bytes --] On Fri, 11 Jun 2021, 15:02 Jan Beulich, <jbeulich@suse.com> wrote: > On 11.06.2021 12:41, Julien Grall wrote: > > On Fri, 11 Jun 2021, 11:16 Jan Beulich, <jbeulich@suse.com> wrote: > > > >> On 11.06.2021 10:00, Julien Grall wrote: > >>> On Fri, 11 Jun 2021, 08:55 Jan Beulich, <jbeulich@suse.com> wrote: > >>> > >>>> The Arm ARM's description of MSR doesn't even allow for plain "SPSR" > >>>> here, and while gas accepts this, it takes it to mean SPSR_cf. Yet > >>>> surely all of SPSR wants updating on this path, not just the lowest > and > >>>> highest 8 bits. > >>>> > >>> > >>> Can you provide a reference to the Arm Arm? This would help to navigate > >>> through the 8000 pages. > >> > >> Referencing the instruction page would be enough, I thought (as > >> even I, not being an Arm person, have no difficulty locating it). > >> If it isn't, how is a canonical doc ref supposed to look like on > >> Arm? On x86, we avoid recording document versions, section > >> numbers, or even page numbers in code comments or commit messages > >> (which isn't to say we have none of these, but we try to avoid > >> new ones to appear), as these tend to change with every new > >> version of the doc. Therefore, to me, the offending commit's "ARM > >> DDI 0487D.b page G8-5993" doesn't look like something I wanted to > >> clone from. But if you tell me otherwise, then well - so be it. > > > > > > The Arm website provides a link for nearly every revision on the specs. > As > > the wording can change between version, it is useful to know which spec > the > > understanding is based from. > > > > Note that for Arm32 we should quote the Armv7 spec and not the Armv8 one > > because we only follow the former (there are a few small differences). > > Thanks for having me dig out an up-to-date Armv7 spec. I find this > puzzling in particular because you didn't care to have the earlier > commit provide a v7 doc ref. Initially I did intentionally use (a > newer version of) the doc that was pointed at there (which I also > think is better structured than the v7 one). Well Stefano replied past midnight UK time with the reference and committed nearly afterwards. So I didn't really have time to object... When I asked for the reference I didn't think I needed to mention it should be the Armv7 as he should know we only support Armv7 for 32-bit. I didn't bother to reply afterwards. But given there is a bug and you quoted him, I chose to make clear that reference should be Armv7 only. Cheers, > Jan > > [-- Attachment #2: Type: text/html, Size: 3787 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-11 13:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-11 7:55 [PATCH] Arm32: MSR to SPSR needs qualification Jan Beulich 2021-06-11 8:00 ` Julien Grall 2021-06-11 9:16 ` Jan Beulich 2021-06-11 10:41 ` Julien Grall 2021-06-11 13:02 ` Jan Beulich 2021-06-11 13:22 ` Julien Grall
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.