Re: [PATCH v3 06/14] xen/asm-generic: introduce generic header percpu.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2023-11-21 at 16:56 +0100, Jan Beulich wrote:
> On 17.11.2023 13:24, Oleksii Kurochko wrote:
> > --- a/xen/arch/ppc/include/asm/current.h
> > +++ b/xen/arch/ppc/include/asm/current.h
> > @@ -4,6 +4,8 @@
> >  
> >  #include <xen/percpu.h>
> >  
> > +#include <asm/processor.h>
> > +
> >  #ifndef __ASSEMBLY__
> >  
> >  struct vcpu;
> > @@ -38,6 +40,10 @@ static inline struct cpu_info
> > *get_cpu_info(void)
> >  
> >  #define guest_cpu_user_regs() (&get_cpu_info()-
> > >guest_cpu_user_regs)
> >  
> > +#define smp_processor_id()      0 /* TODO: Fix this */
> > +
> > +#define get_per_cpu_offset()    smp_processor_id()
> 
> This 2nd item can't be quite right either, so likely also wants a
> FIXME comment.
> Shawn will have the ultimate say here anyway.
I did so because it is how percpu stuff was implemented before
get_per_cpu_offset was introduced.

> 
> > --- a/xen/include/xen/percpu.h
> > +++ b/xen/include/xen/percpu.h
> > @@ -1,8 +1,6 @@
> >  #ifndef __XEN_PERCPU_H__
> >  #define __XEN_PERCPU_H__
> >  
> > -#include <asm/percpu.h>
> > -
> >  #define DECLARE_PER_CPU(type, name) \
> >      extern __typeof__(type) per_cpu__ ## name
> >  
> > @@ -29,6 +27,8 @@
> >  
> >  #define get_per_cpu_var(var)  (per_cpu__##var)
> >  
> > +#include <asm/percpu.h>
> > +
> >  /* Linux compatibility. */
> >  #define get_cpu_var(var) this_cpu(var)
> >  #define put_cpu_var(var)
> 
> While you explain the reason for this movement, it still feels a
> little
> fragile to me.

The reason for that is #include <asm/processor.h> was added to
<asm/percpu.h>. <asm/processor.h> uses DECLARE_PER_CPU(...) so it
should be defined before inclusion of <asm/percpu.h>. Otherwise the
following error will occur:
./arch/riscv/include/asm/current.h:13:32: error: unknown type name
'curr_vcpu'
   13 | DECLARE_PER_CPU(struct vcpu *, curr_vcpu);                    
      |                                ^~~~~~~~~        
In file included from ././include/xen/config.h:17,
                 from <command-line>:                              
./include/xen/sched.h: In function 'rcu_unlock_domain':
./include/asm-generic/percpu.h:19:19: error: 'per_cpu__curr_vcpu'
undeclared (first use in this function)
   19 |     (*RELOC_HIDE(&per_cpu__##var, get_per_cpu_offset()))
      |                   ^~~~~~~~~                                   
./include/xen/compiler.h:146:37: note: in definition of macro
'RELOC_HIDE'
  146 |     __asm__ ("" : "=r"(__ptr) : "0"(ptr));      \
      |                                     ^~~                       
./arch/riscv/include/asm/current.h:15:29: note: in expansion of macro
'this_cpu'
   15 | #define current            (this_cpu(curr_vcpu))
      |                             ^~~~~~~~                          
./include/xen/sched.h:726:15: note: in expansion of macro 'current'
  726 |     if ( d != current->domain )     
      |               ^~~~~~~                                      
./include/asm-generic/percpu.h:19:19: note: each undeclared identifier
is reported only once for each function it appears in
   19 |     (*RELOC_HIDE(&per_cpu__##var, get_per_cpu_offset()))
      |                   ^~~~~~~~~              
./include/xen/compiler.h:146:37: note: in definition of macro
'RELOC_HIDE'
  146 |     __asm__ ("" : "=r"(__ptr) : "0"(ptr));      \
      |                                     ^~~
./arch/riscv/include/asm/current.h:15:29: note: in expansion of macro
'this_cpu'
   15 | #define current            (this_cpu(curr_vcpu))
      |                             ^~~~~~~~
./include/xen/sched.h:726:15: note: in expansion of macro 'current'
  726 |     if ( d != current->domain )

~ Oleksii