[PATCH v12 7/8] xen/riscv: enable full Xen build

Oleksii Kurochko posted 8 patches 5 months, 4 weeks ago
[PATCH v12 7/8] xen/riscv: enable full Xen build
Posted by Oleksii Kurochko 5 months, 4 weeks ago
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 At least this patch cann't be merged w/o Andrew's patch series is merged as ffs related
 functions are used from that patch series:
  https://lore.kernel.org/xen-devel/20240313172716.2325427-1-andrew.cooper3@citrix.com/T/#t
---
Changes in V5-V12:
 - Nothing changed. Only rebase.
 - Add the footer after Signed-off section.
---
Changes in V4:
 - drop stubs for irq_actor_none() and irq_actor_none() as common/irq.c is compiled now.
 - drop defintion of max_page in stubs.c as common/page_alloc.c is compiled now.
 - drop printk() related changes in riscv/early_printk.c as common version will be used.
---
Changes in V3:
 - Reviewed-by: Jan Beulich <jbeulich@suse.com>
 - unrealted change dropped in tiny64_defconfig
---
Changes in V2:
 - Nothing changed. Only rebase.
---
 xen/arch/riscv/Makefile       |  16 +++-
 xen/arch/riscv/arch.mk        |   4 -
 xen/arch/riscv/early_printk.c | 168 ----------------------------------
 xen/arch/riscv/stubs.c        |  24 -----
 4 files changed, 15 insertions(+), 197 deletions(-)

diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile
index 60afbc0ad9..81b77b13d6 100644
--- a/xen/arch/riscv/Makefile
+++ b/xen/arch/riscv/Makefile
@@ -12,10 +12,24 @@ $(TARGET): $(TARGET)-syms
 	$(OBJCOPY) -O binary -S $< $@
 
 $(TARGET)-syms: $(objtree)/prelink.o $(obj)/xen.lds
-	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) -o $@
+	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< \
+	    $(objtree)/common/symbols-dummy.o -o $(dot-target).0
+	$(NM) -pa --format=sysv $(dot-target).0 \
+		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
+		> $(dot-target).0.S
+	$(MAKE) $(build)=$(@D) $(dot-target).0.o
+	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< \
+	    $(dot-target).0.o -o $(dot-target).1
+	$(NM) -pa --format=sysv $(dot-target).1 \
+		| $(objtree)/tools/symbols $(all_symbols) --sysv --sort \
+		> $(dot-target).1.S
+	$(MAKE) $(build)=$(@D) $(dot-target).1.o
+	$(LD) $(XEN_LDFLAGS) -T $(obj)/xen.lds -N $< $(build_id_linker) \
+	    $(dot-target).1.o -o $@
 	$(NM) -pa --format=sysv $@ \
 		| $(objtree)/tools/symbols --all-symbols --xensyms --sysv --sort \
 		> $@.map
+	rm -f $(@D)/.$(@F).[0-9]*
 
 $(obj)/xen.lds: $(src)/xen.lds.S FORCE
 	$(call if_changed_dep,cpp_lds_S)
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 8c071aff65..17827c302c 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -38,7 +38,3 @@ extensions := $(subst $(space),,$(extensions))
 # -mcmodel=medlow would force Xen into the lower half.
 
 CFLAGS += $(riscv-generic-flags)$(extensions) -mstrict-align -mcmodel=medany
-
-# TODO: Drop override when more of the build is working
-override ALL_OBJS-y = arch/$(SRCARCH)/built_in.o
-override ALL_LIBS-y =
diff --git a/xen/arch/riscv/early_printk.c b/xen/arch/riscv/early_printk.c
index 60742a042d..610c814f54 100644
--- a/xen/arch/riscv/early_printk.c
+++ b/xen/arch/riscv/early_printk.c
@@ -40,171 +40,3 @@ void early_printk(const char *str)
         str++;
     }
 }
-
-/*
- * The following #if 1 ... #endif should be removed after printk
- * and related stuff are ready.
- */
-#if 1
-
-#include <xen/stdarg.h>
-#include <xen/string.h>
-
-/**
- * strlen - Find the length of a string
- * @s: The string to be sized
- */
-size_t (strlen)(const char * s)
-{
-    const char *sc;
-
-    for (sc = s; *sc != '\0'; ++sc)
-        /* nothing */;
-    return sc - s;
-}
-
-/**
- * memcpy - Copy one area of memory to another
- * @dest: Where to copy to
- * @src: Where to copy from
- * @count: The size of the area.
- *
- * You should not use this function to access IO space, use memcpy_toio()
- * or memcpy_fromio() instead.
- */
-void *(memcpy)(void *dest, const void *src, size_t count)
-{
-    char *tmp = (char *) dest, *s = (char *) src;
-
-    while (count--)
-        *tmp++ = *s++;
-
-    return dest;
-}
-
-int vsnprintf(char* str, size_t size, const char* format, va_list args)
-{
-    size_t i = 0; /* Current position in the output string */
-    size_t written = 0; /* Total number of characters written */
-    char* dest = str;
-
-    while ( format[i] != '\0' && written < size - 1 )
-    {
-        if ( format[i] == '%' )
-        {
-            i++;
-
-            if ( format[i] == '\0' )
-                break;
-
-            if ( format[i] == '%' )
-            {
-                if ( written < size - 1 )
-                {
-                    dest[written] = '%';
-                    written++;
-                }
-                i++;
-                continue;
-            }
-
-            /*
-             * Handle format specifiers.
-             * For simplicity, only %s and %d are implemented here.
-             */
-
-            if ( format[i] == 's' )
-            {
-                char* arg = va_arg(args, char*);
-                size_t arglen = strlen(arg);
-
-                size_t remaining = size - written - 1;
-
-                if ( arglen > remaining )
-                    arglen = remaining;
-
-                memcpy(dest + written, arg, arglen);
-
-                written += arglen;
-                i++;
-            }
-            else if ( format[i] == 'd' )
-            {
-                int arg = va_arg(args, int);
-
-                /* Convert the integer to string representation */
-                char numstr[32]; /* Assumes a maximum of 32 digits */
-                int numlen = 0;
-                int num = arg;
-                size_t remaining;
-
-                if ( arg < 0 )
-                {
-                    if ( written < size - 1 )
-                    {
-                        dest[written] = '-';
-                        written++;
-                    }
-
-                    num = -arg;
-                }
-
-                do
-                {
-                    numstr[numlen] = '0' + num % 10;
-                    num = num / 10;
-                    numlen++;
-                } while ( num > 0 );
-
-                /* Reverse the string */
-                for (int j = 0; j < numlen / 2; j++)
-                {
-                    char tmp = numstr[j];
-                    numstr[j] = numstr[numlen - 1 - j];
-                    numstr[numlen - 1 - j] = tmp;
-                }
-
-                remaining = size - written - 1;
-
-                if ( numlen > remaining )
-                    numlen = remaining;
-
-                memcpy(dest + written, numstr, numlen);
-
-                written += numlen;
-                i++;
-            }
-        }
-        else
-        {
-            if ( written < size - 1 )
-            {
-                dest[written] = format[i];
-                written++;
-            }
-            i++;
-        }
-    }
-
-    if ( size > 0 )
-        dest[written] = '\0';
-
-    return written;
-}
-
-void printk(const char *format, ...)
-{
-    static char buf[1024];
-
-    va_list args;
-    va_start(args, format);
-
-    (void)vsnprintf(buf, sizeof(buf), format, args);
-
-    early_printk(buf);
-
-    va_end(args);
-}
-
-#endif
-
diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
index 8285bcffef..bda35fc347 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -24,12 +24,6 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
 
 nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
 
-/*
- * max_page is defined in page_alloc.c which isn't complied for now.
- * definition of max_page will be remove as soon as page_alloc is built.
- */
-unsigned long __read_mostly max_page;
-
 /* time.c */
 
 unsigned long __ro_after_init cpu_khz;  /* CPU clock frequency in kHz. */
@@ -419,21 +413,3 @@ void __cpu_die(unsigned int cpu)
 {
     BUG_ON("unimplemented");
 }
-
-/*
- * The following functions are defined in common/irq.c, but common/irq.c isn't
- * built for now. These changes will be removed there when common/irq.c is
- * ready.
- */
-
-void cf_check irq_actor_none(struct irq_desc *desc)
-{
-    BUG_ON("unimplemented");
-}
-
-unsigned int cf_check irq_startup_none(struct irq_desc *desc)
-{
-    BUG_ON("unimplemented");
-
-    return 0;
-}
-- 
2.45.0
Re: [PATCH v12 7/8] xen/riscv: enable full Xen build
Posted by Andrew Cooper 5 months, 3 weeks ago
On 29/05/2024 8:55 pm, Oleksii Kurochko wrote:
> diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
> index 8285bcffef..bda35fc347 100644
> --- a/xen/arch/riscv/stubs.c
> +++ b/xen/arch/riscv/stubs.c
> @@ -24,12 +24,6 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
>  
>  nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
>  
> -/*
> - * max_page is defined in page_alloc.c which isn't complied for now.
> - * definition of max_page will be remove as soon as page_alloc is built.
> - */
> -unsigned long __read_mostly max_page;
> -
>  /* time.c */
>  
>  unsigned long __ro_after_init cpu_khz;  /* CPU clock frequency in kHz. */
> @@ -419,21 +413,3 @@ void __cpu_die(unsigned int cpu)
>  {
>      BUG_ON("unimplemented");
>  }
> -
> -/*
> - * The following functions are defined in common/irq.c, but common/irq.c isn't
> - * built for now. These changes will be removed there when common/irq.c is
> - * ready.
> - */
> -
> -void cf_check irq_actor_none(struct irq_desc *desc)
> -{
> -    BUG_ON("unimplemented");
> -}
> -
> -unsigned int cf_check irq_startup_none(struct irq_desc *desc)
> -{
> -    BUG_ON("unimplemented");
> -
> -    return 0;
> -}

All 3 of these are introduced in the previous patch and deleted again
here.  Looks like a rebasing accident.

(This patch has to be the final one touching build related things, so I
can't simply fix it on commit)

~Andrew

Re: [PATCH v12 7/8] xen/riscv: enable full Xen build
Posted by Oleksii K. 5 months, 3 weeks ago
On Thu, 2024-05-30 at 17:48 +0100, Andrew Cooper wrote:
> On 29/05/2024 8:55 pm, Oleksii Kurochko wrote:
> > diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
> > index 8285bcffef..bda35fc347 100644
> > --- a/xen/arch/riscv/stubs.c
> > +++ b/xen/arch/riscv/stubs.c
> > @@ -24,12 +24,6 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t,
> > cpu_core_mask);
> >  
> >  nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
> >  
> > -/*
> > - * max_page is defined in page_alloc.c which isn't complied for
> > now.
> > - * definition of max_page will be remove as soon as page_alloc is
> > built.
> > - */
> > -unsigned long __read_mostly max_page;
> > -
> >  /* time.c */
> >  
> >  unsigned long __ro_after_init cpu_khz;  /* CPU clock frequency in
> > kHz. */
> > @@ -419,21 +413,3 @@ void __cpu_die(unsigned int cpu)
> >  {
> >      BUG_ON("unimplemented");
> >  }
> > -
> > -/*
> > - * The following functions are defined in common/irq.c, but
> > common/irq.c isn't
> > - * built for now. These changes will be removed there when
> > common/irq.c is
> > - * ready.
> > - */
> > -
> > -void cf_check irq_actor_none(struct irq_desc *desc)
> > -{
> > -    BUG_ON("unimplemented");
> > -}
> > -
> > -unsigned int cf_check irq_startup_none(struct irq_desc *desc)
> > -{
> > -    BUG_ON("unimplemented");
> > -
> > -    return 0;
> > -}
> 
> All 3 of these are introduced in the previous patch and deleted again
> here.  Looks like a rebasing accident.
Not really.

This was done to avoid build failures for RISC-V. If the HEAD is on the
previous patch where these changes are introduced and then we just drop
them, it will lead to a linkage error because these functions are
defined in common/irq.c, which isn't built for RISC-V if the HEAD is on
the previous patch:
   /build/xen/arch/riscv/entry.S:86: undefined reference to `max_page'
   riscv64-linux-gnu-ld: prelink.o:(.rodata+0x8): undefined reference to
   `irq_startup_none'
   riscv64-linux-gnu-ld: prelink.o:(.rodata+0x10): undefined reference to
   `irq_actor_none'
   riscv64-linux-gnu-ld: prelink.o:(.rodata+0x18): undefined reference to
   `irq_actor_none'
   riscv64-linux-gnu-ld: prelink.o:(.rodata+0x20): undefined reference to
   `irq_actor_none'
   riscv64-linux-gnu-ld: xen-syms: hidden symbol `irq_actor_none' isn't
   defined

That is why these stubs were introduced in the previous patch (because
common/irq.c isn't built at that moment) and are removed in this patch
(since at the moment of this patch, common/irq.c is now being built).

~ Oleksii

> 
> (This patch has to be the final one touching build related things, so
> I
> can't simply fix it on commit)
> 
> ~Andrew
Re: [PATCH v12 7/8] xen/riscv: enable full Xen build
Posted by Andrew Cooper 5 months, 3 weeks ago
On 30/05/2024 6:12 pm, Oleksii K. wrote:
> On Thu, 2024-05-30 at 17:48 +0100, Andrew Cooper wrote:
>> On 29/05/2024 8:55 pm, Oleksii Kurochko wrote:
>>> diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
>>> index 8285bcffef..bda35fc347 100644
>>> --- a/xen/arch/riscv/stubs.c
>>> +++ b/xen/arch/riscv/stubs.c
>>> @@ -24,12 +24,6 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t,
>>> cpu_core_mask);
>>>  
>>>  nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
>>>  
>>> -/*
>>> - * max_page is defined in page_alloc.c which isn't complied for
>>> now.
>>> - * definition of max_page will be remove as soon as page_alloc is
>>> built.
>>> - */
>>> -unsigned long __read_mostly max_page;
>>> -
>>>  /* time.c */
>>>  
>>>  unsigned long __ro_after_init cpu_khz;  /* CPU clock frequency in
>>> kHz. */
>>> @@ -419,21 +413,3 @@ void __cpu_die(unsigned int cpu)
>>>  {
>>>      BUG_ON("unimplemented");
>>>  }
>>> -
>>> -/*
>>> - * The following functions are defined in common/irq.c, but
>>> common/irq.c isn't
>>> - * built for now. These changes will be removed there when
>>> common/irq.c is
>>> - * ready.
>>> - */
>>> -
>>> -void cf_check irq_actor_none(struct irq_desc *desc)
>>> -{
>>> -    BUG_ON("unimplemented");
>>> -}
>>> -
>>> -unsigned int cf_check irq_startup_none(struct irq_desc *desc)
>>> -{
>>> -    BUG_ON("unimplemented");
>>> -
>>> -    return 0;
>>> -}
>> All 3 of these are introduced in the previous patch and deleted again
>> here.  Looks like a rebasing accident.
> Not really.
>
> This was done to avoid build failures for RISC-V. If the HEAD is on the
> previous patch where these changes are introduced and then we just drop
> them, it will lead to a linkage error because these functions are
> defined in common/irq.c, which isn't built for RISC-V if the HEAD is on
> the previous patch:
>    /build/xen/arch/riscv/entry.S:86: undefined reference to `max_page'

Nothing in the series touches entry.S, so I'm not sure what this is
complaining about.

The stub for get_upper_mfn_bound() references max_page, but it's only
used in a SYSCTL so you can avoid the problem with a BUG_ON().

BTW, you also don't need a return after a BUG_ON(). 
__builtin_unreachable() takes care of everything properly for you.


>    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x8): undefined reference to
>    `irq_startup_none'
>    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x10): undefined reference to
>    `irq_actor_none'
>    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x18): undefined reference to
>    `irq_actor_none'
>    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x20): undefined reference to
>    `irq_actor_none'
>    riscv64-linux-gnu-ld: xen-syms: hidden symbol `irq_actor_none' isn't
>    defined
>
> That is why these stubs were introduced in the previous patch (because
> common/irq.c isn't built at that moment) and are removed in this patch
> (since at the moment of this patch, common/irq.c is now being built).

These OTOH are a side effect of how no_irq_type works, which is
horrifying, and not something I can unsee.

I'll see about fixing it, because I really can't bare to leave it like
this...

~Andrew

Re: [PATCH v12 7/8] xen/riscv: enable full Xen build
Posted by Oleksii K. 5 months, 3 weeks ago
On Thu, 2024-05-30 at 18:45 +0100, Andrew Cooper wrote:
> On 30/05/2024 6:12 pm, Oleksii K. wrote:
> > On Thu, 2024-05-30 at 17:48 +0100, Andrew Cooper wrote:
> > > On 29/05/2024 8:55 pm, Oleksii Kurochko wrote:
> > > > diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
> > > > index 8285bcffef..bda35fc347 100644
> > > > --- a/xen/arch/riscv/stubs.c
> > > > +++ b/xen/arch/riscv/stubs.c
> > > > @@ -24,12 +24,6 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t,
> > > > cpu_core_mask);
> > > >  
> > > >  nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
> > > >  
> > > > -/*
> > > > - * max_page is defined in page_alloc.c which isn't complied
> > > > for
> > > > now.
> > > > - * definition of max_page will be remove as soon as page_alloc
> > > > is
> > > > built.
> > > > - */
> > > > -unsigned long __read_mostly max_page;
> > > > -
> > > >  /* time.c */
> > > >  
> > > >  unsigned long __ro_after_init cpu_khz;  /* CPU clock frequency
> > > > in
> > > > kHz. */
> > > > @@ -419,21 +413,3 @@ void __cpu_die(unsigned int cpu)
> > > >  {
> > > >      BUG_ON("unimplemented");
> > > >  }
> > > > -
> > > > -/*
> > > > - * The following functions are defined in common/irq.c, but
> > > > common/irq.c isn't
> > > > - * built for now. These changes will be removed there when
> > > > common/irq.c is
> > > > - * ready.
> > > > - */
> > > > -
> > > > -void cf_check irq_actor_none(struct irq_desc *desc)
> > > > -{
> > > > -    BUG_ON("unimplemented");
> > > > -}
> > > > -
> > > > -unsigned int cf_check irq_startup_none(struct irq_desc *desc)
> > > > -{
> > > > -    BUG_ON("unimplemented");
> > > > -
> > > > -    return 0;
> > > > -}
> > > All 3 of these are introduced in the previous patch and deleted
> > > again
> > > here.  Looks like a rebasing accident.
> > Not really.
> > 
> > This was done to avoid build failures for RISC-V. If the HEAD is on
> > the
> > previous patch where these changes are introduced and then we just
> > drop
> > them, it will lead to a linkage error because these functions are
> > defined in common/irq.c, which isn't built for RISC-V if the HEAD
> > is on
> > the previous patch:
> >    /build/xen/arch/riscv/entry.S:86: undefined reference to
> > `max_page'
> 
> Nothing in the series touches entry.S, so I'm not sure what this is
> complaining about.
> 
> The stub for get_upper_mfn_bound() references max_page, but it's only
> used in a SYSCTL so you can avoid the problem with a BUG_ON().
I didn't get how I can use BUG_ON() with declaration of variable to
avoid the compilation issue the undefined reference to max_page?

> 
> BTW, you also don't need a return after a BUG_ON(). 
> __builtin_unreachable() takes care of everything properly for you.
> 
> 
> >    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x8): undefined
> > reference to
> >    `irq_startup_none'
> >    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x10): undefined
> > reference to
> >    `irq_actor_none'
> >    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x18): undefined
> > reference to
> >    `irq_actor_none'
> >    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x20): undefined
> > reference to
> >    `irq_actor_none'
> >    riscv64-linux-gnu-ld: xen-syms: hidden symbol `irq_actor_none'
> > isn't
> >    defined
> > 
> > That is why these stubs were introduced in the previous patch
> > (because
> > common/irq.c isn't built at that moment) and are removed in this
> > patch
> > (since at the moment of this patch, common/irq.c is now being
> > built).
> 
> These OTOH are a side effect of how no_irq_type works, which is
> horrifying, and not something I can unsee.
> 
> I'll see about fixing it, because I really can't bare to leave it
> like
> this...
I am really sorry, but I don't understand should I deal with this
somehow now or the provided changes are okay?

~ Oleksii
Re: [PATCH v12 7/8] xen/riscv: enable full Xen build
Posted by Andrew Cooper 5 months, 3 weeks ago
On 30/05/2024 7:27 pm, Oleksii K. wrote:
> On Thu, 2024-05-30 at 18:45 +0100, Andrew Cooper wrote:
>> On 30/05/2024 6:12 pm, Oleksii K. wrote:
>>> On Thu, 2024-05-30 at 17:48 +0100, Andrew Cooper wrote:
>>>> On 29/05/2024 8:55 pm, Oleksii Kurochko wrote:
>>>>> diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
>>>>> index 8285bcffef..bda35fc347 100644
>>>>> --- a/xen/arch/riscv/stubs.c
>>>>> +++ b/xen/arch/riscv/stubs.c
>>>>> @@ -24,12 +24,6 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t,
>>>>> cpu_core_mask);
>>>>>  
>>>>>  nodemask_t __read_mostly node_online_map = { { [0] = 1UL } };
>>>>>  
>>>>> -/*
>>>>> - * max_page is defined in page_alloc.c which isn't complied
>>>>> for
>>>>> now.
>>>>> - * definition of max_page will be remove as soon as page_alloc
>>>>> is
>>>>> built.
>>>>> - */
>>>>> -unsigned long __read_mostly max_page;
>>>>> -
>>>>>  /* time.c */
>>>>>  
>>>>>  unsigned long __ro_after_init cpu_khz;  /* CPU clock frequency
>>>>> in
>>>>> kHz. */
>>>>> @@ -419,21 +413,3 @@ void __cpu_die(unsigned int cpu)
>>>>>  {
>>>>>      BUG_ON("unimplemented");
>>>>>  }
>>>>> -
>>>>> -/*
>>>>> - * The following functions are defined in common/irq.c, but
>>>>> common/irq.c isn't
>>>>> - * built for now. These changes will be removed there when
>>>>> common/irq.c is
>>>>> - * ready.
>>>>> - */
>>>>> -
>>>>> -void cf_check irq_actor_none(struct irq_desc *desc)
>>>>> -{
>>>>> -    BUG_ON("unimplemented");
>>>>> -}
>>>>> -
>>>>> -unsigned int cf_check irq_startup_none(struct irq_desc *desc)
>>>>> -{
>>>>> -    BUG_ON("unimplemented");
>>>>> -
>>>>> -    return 0;
>>>>> -}
>>>> All 3 of these are introduced in the previous patch and deleted
>>>> again
>>>> here.  Looks like a rebasing accident.
>>> Not really.
>>>
>>> This was done to avoid build failures for RISC-V. If the HEAD is on
>>> the
>>> previous patch where these changes are introduced and then we just
>>> drop
>>> them, it will lead to a linkage error because these functions are
>>> defined in common/irq.c, which isn't built for RISC-V if the HEAD
>>> is on
>>> the previous patch:
>>>    /build/xen/arch/riscv/entry.S:86: undefined reference to
>>> `max_page'
>> Nothing in the series touches entry.S, so I'm not sure what this is
>> complaining about.
>>
>> The stub for get_upper_mfn_bound() references max_page, but it's only
>> used in a SYSCTL so you can avoid the problem with a BUG_ON().
> I didn't get how I can use BUG_ON() with declaration of variable to
> avoid the compilation issue the undefined reference to max_page?

You don't need to implement get_upper_mfn_bound() yet.

If you implement it as:

unsigned long get_upper_mfn_bound(void)
{
    BUG_ON("unimplemented");
}

in the previous patch, then you also don't have any problems with
max_page between these two patches either.

>
>> BTW, you also don't need a return after a BUG_ON(). 
>> __builtin_unreachable() takes care of everything properly for you.
>>
>>
>>>    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x8): undefined
>>> reference to
>>>    `irq_startup_none'
>>>    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x10): undefined
>>> reference to
>>>    `irq_actor_none'
>>>    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x18): undefined
>>> reference to
>>>    `irq_actor_none'
>>>    riscv64-linux-gnu-ld: prelink.o:(.rodata+0x20): undefined
>>> reference to
>>>    `irq_actor_none'
>>>    riscv64-linux-gnu-ld: xen-syms: hidden symbol `irq_actor_none'
>>> isn't
>>>    defined
>>>
>>> That is why these stubs were introduced in the previous patch
>>> (because
>>> common/irq.c isn't built at that moment) and are removed in this
>>> patch
>>> (since at the moment of this patch, common/irq.c is now being
>>> built).
>> These OTOH are a side effect of how no_irq_type works, which is
>> horrifying, and not something I can unsee.
>>
>> I'll see about fixing it, because I really can't bare to leave it
>> like
>> this...
> I am really sorry, but I don't understand should I deal with this
> somehow now or the provided changes are okay?

I've emailed out a 2-patch series to unbreak this.

~Andrew