They're all within a 12 bit range of their respective bases, and this prevents
all the MSR coordinates being calculated in %rcx.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
There's one unpleasant surprise hidden:
add/remove: 0/0 grow/shrink: 2/6 up/down: 18/-99 (-81)
Function old new delta
trace_exit_reason.part 229 245 +16
check_for_unexpected_msi 73 75 +2
fixup_irqs 1239 1237 -2
lapic_resume 860 844 -16
irq_move_cleanup_interrupt 683 667 -16
intel_mcheck_init 1840 1824 -16
setup_local_APIC 985 968 -17
clear_local_APIC 1141 1109 -32
This causes check_for_unexpected_msi() to gain a CLTQ sign extending reg
before adding it to APIC_BASE. Furthermore it retains it's SAR from the start
of apic_isr_read().
If the vector parameter changes from uint8_t to unsigned int, both the CLTQ
and SAR go away and and are replaced with regular unsigned logic.
(uint8_t) & ~0x1f undergoes promotion to int, but can't be negative due to
it's range, and I'm quite surprised that GCC-12 can't figure this out.
---
xen/arch/x86/include/asm/apic.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/include/asm/apic.h b/xen/arch/x86/include/asm/apic.h
index a254e49dd13b..1133954e5540 100644
--- a/xen/arch/x86/include/asm/apic.h
+++ b/xen/arch/x86/include/asm/apic.h
@@ -49,12 +49,12 @@ const struct genapic *apic_x2apic_probe(void);
* Basic functions accessing APICs.
*/
-static inline void apic_mem_write(unsigned long reg, u32 v)
+static inline void apic_mem_write(unsigned int reg, u32 v)
{
*((volatile u32 *)(APIC_BASE+reg)) = v;
}
-static inline u32 apic_mem_read(unsigned long reg)
+static inline u32 apic_mem_read(unsigned int reg)
{
return *((volatile u32 *)(APIC_BASE+reg));
}
@@ -63,7 +63,7 @@ static inline u32 apic_mem_read(unsigned long reg)
* access the 64-bit ICR register.
*/
-static inline void apic_wrmsr(unsigned long reg, uint64_t msr_content)
+static inline void apic_wrmsr(unsigned int reg, uint64_t msr_content)
{
if (reg == APIC_DFR || reg == APIC_ID || reg == APIC_LDR ||
reg == APIC_LVR)
@@ -72,7 +72,7 @@ static inline void apic_wrmsr(unsigned long reg, uint64_t msr_content)
wrmsrl(MSR_X2APIC_FIRST + (reg >> 4), msr_content);
}
-static inline uint64_t apic_rdmsr(unsigned long reg)
+static inline uint64_t apic_rdmsr(unsigned int reg)
{
uint64_t msr_content;
@@ -83,7 +83,7 @@ static inline uint64_t apic_rdmsr(unsigned long reg)
return msr_content;
}
-static inline void apic_write(unsigned long reg, u32 v)
+static inline void apic_write(unsigned int reg, u32 v)
{
if ( x2apic_enabled )
@@ -92,7 +92,7 @@ static inline void apic_write(unsigned long reg, u32 v)
apic_mem_write(reg, v);
}
-static inline u32 apic_read(unsigned long reg)
+static inline u32 apic_read(unsigned int reg)
{
if ( x2apic_enabled )
return apic_rdmsr(reg);
--
2.39.2
On 23.07.2024 23:05, Andrew Cooper wrote: > They're all within a 12 bit range of their respective bases, and this prevents > all the MSR coordinates being calculated in %rcx. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> > There's one unpleasant surprise hidden: > > add/remove: 0/0 grow/shrink: 2/6 up/down: 18/-99 (-81) > Function old new delta > trace_exit_reason.part 229 245 +16 > check_for_unexpected_msi 73 75 +2 > fixup_irqs 1239 1237 -2 > lapic_resume 860 844 -16 > irq_move_cleanup_interrupt 683 667 -16 > intel_mcheck_init 1840 1824 -16 > setup_local_APIC 985 968 -17 > clear_local_APIC 1141 1109 -32 > > This causes check_for_unexpected_msi() to gain a CLTQ sign extending reg > before adding it to APIC_BASE. Furthermore it retains it's SAR from the start > of apic_isr_read(). > > If the vector parameter changes from uint8_t to unsigned int, both the CLTQ > and SAR go away and and are replaced with regular unsigned logic. > > (uint8_t) & ~0x1f undergoes promotion to int, but can't be negative due to > it's range, and I'm quite surprised that GCC-12 can't figure this out. Odd indeed. But certainly no the only odd thing in code generation. > --- a/xen/arch/x86/include/asm/apic.h > +++ b/xen/arch/x86/include/asm/apic.h > @@ -49,12 +49,12 @@ const struct genapic *apic_x2apic_probe(void); > * Basic functions accessing APICs. > */ > > -static inline void apic_mem_write(unsigned long reg, u32 v) > +static inline void apic_mem_write(unsigned int reg, u32 v) Mind me asking that on lines you touch anyway you also change u32 to uint32_t? Jan
© 2016 - 2024 Red Hat, Inc.