While a number of issues we previously had with pre-release gcc12 were
fixed in the final release, we continue to have one issue (with multiple
instances) when doing release builds (i.e. at higher optimization
levels): The compiler takes issue with subtracting (always 1 in our
case) from artifical labels (expressed as array) marking the end of
certain regions. This isn't an unreasonable position to take. Simply
hide the "array-ness" by casting to an integer type. To keep things
looking consistently, apply the same cast also on the respective
expressions dealing with the starting addresses. (Note how
efi_arch_memory_setup()'s l2_table_offset() invocations avoid a similar
issue by already having the necessary casts.) In is_xen_fixed_mfn()
further switch from __pa() to virt_to_maddr() to better match the left
sides of the <= operators.

Reported-by: Charles Arnold <carnold@suse.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Initially I had considered introducing something like END_MINUS_1(), but
in the end I did consider this uglier than explicitly dealing with the
two instances we have.

--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -677,10 +677,10 @@ static void __init efi_arch_memory_setup
      * appropriate l2 slots to map.
      */
 #define l2_4G_offset(a)                                                 \
-    (((UINTN)(a) >> L2_PAGETABLE_SHIFT) & (4 * L2_PAGETABLE_ENTRIES - 1))
+    (((a) >> L2_PAGETABLE_SHIFT) & (4 * L2_PAGETABLE_ENTRIES - 1))
 
-    for ( i  = l2_4G_offset(_start);
-          i <= l2_4G_offset(_end - 1); ++i )
+    for ( i  = l2_4G_offset((UINTN)_start);
+          i <= l2_4G_offset((UINTN)_end - 1); ++i )
     {
         l2_pgentry_t pte = l2e_from_paddr(i << L2_PAGETABLE_SHIFT,
                                           __PAGE_HYPERVISOR | _PAGE_PSE);
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -309,8 +309,8 @@ struct page_info
 #define is_xen_heap_mfn(mfn) \
     (mfn_valid(mfn) && is_xen_heap_page(mfn_to_page(mfn)))
 #define is_xen_fixed_mfn(mfn)                     \
-    (((mfn_to_maddr(mfn)) >= __pa(_stext)) &&     \
-     ((mfn_to_maddr(mfn)) <= __pa(__2M_rwdata_end - 1)))
+    (((mfn_to_maddr(mfn)) >= virt_to_maddr((unsigned long)_stext)) && \
+     ((mfn_to_maddr(mfn)) <= virt_to_maddr((unsigned long)__2M_rwdata_end - 1)))
 
 #define PRtype_info "016lx"/* should only be used for printk's */

On 14/07/2022 08:53, Jan Beulich wrote:
> While a number of issues we previously had with pre-release gcc12 were
> fixed in the final release, we continue to have one issue (with multiple
> instances) when doing release builds (i.e. at higher optimization
> levels): The compiler takes issue with subtracting (always 1 in our
> case) from artifical labels (expressed as array) marking the end of
> certain regions. This isn't an unreasonable position to take. Simply
> hide the "array-ness" by casting to an integer type. To keep things
> looking consistently, apply the same cast also on the respective
> expressions dealing with the starting addresses. (Note how
> efi_arch_memory_setup()'s l2_table_offset() invocations avoid a similar
> issue by already having the necessary casts.) In is_xen_fixed_mfn()
> further switch from __pa() to virt_to_maddr() to better match the left
> sides of the <= operators.
>
> Reported-by: Charles Arnold <carnold@suse.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Initially I had considered introducing something like END_MINUS_1(), but
> in the end I did consider this uglier than explicitly dealing with the
> two instances we have.

Yeah, I prefer this form.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>