subject:"RE\: \[PATCH 2\/5\] X86\: Hyper\-V\: Enable IPI enlightenments"

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread kbuild test robot

Hi Srinivasan,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.17-rc2]
[also build test ERROR on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
config: i386-randconfig-s1-04271426 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_read':
   arch/x86/hyperv/hv_apic.c:73:10: error: implicit declaration of function 
'native_apic_mem_read' [-Werror=implicit-function-declaration]
  return native_apic_mem_read(reg);
 ^~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_write':
   arch/x86/hyperv/hv_apic.c:87:3: error: implicit declaration of function 
'native_apic_mem_write' [-Werror=implicit-function-declaration]
  native_apic_mem_write(reg, val);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi':
   arch/x86/hyperv/hv_apic.c:155:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI(cpu, vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_mask':
   arch/x86/hyperv/hv_apic.c:161:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_mask(mask, vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_mask_allbutself':
   arch/x86/hyperv/hv_apic.c:174:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_mask_allbutself(mask, vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_all':
   arch/x86/hyperv/hv_apic.c:185:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_all(vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_self':
   arch/x86/hyperv/hv_apic.c:191:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_self(vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_init':
   arch/x86/hyperv/hv_apic.c:204:16: error: 'apic' undeclared (first use in 
this function)
  orig_apic = *apic;
   ^~~~
   arch/x86/hyperv/hv_apic.c:204:16: note: each undeclared identifier is 
reported only once for each function it appears in
>> arch/x86/hyperv/hv_apic.c:204:3: error: 'orig_apic' has an incomplete type 
>> 'struct apic'
  orig_apic = *apic;
  ^
   arch/x86/hyperv/hv_apic.c:217:3: error: implicit declaration of function 
'apic_set_eoi_write' [-Werror=implicit-function-declaration]
  apic_set_eoi_write(hv_apic_eoi_write);
  ^~
   arch/x86/hyperv/hv_apic.c: At top level:
>> arch/x86/hyperv/hv_apic.c:39:20: error: storage size of 'orig_apic' isn't 
>> known
static struct apic orig_apic;
   ^
   cc1: some warnings being treated as errors

vim +204 arch/x86/hyperv/hv_apic.c

38  
  > 39  static struct apic orig_apic;
40  
41  static u64 hv_apic_icr_read(void)
42  {
43  u64 reg_val;
44  
45  rdmsrl(HV_X64_MSR_ICR, reg_val);
46  return reg_val;
47  }
48  
49  static void hv_apic_icr_write(u32 low, u32 id)
50  {
51  u64 reg_val;
52  
53  reg_val = SET_APIC_DEST_FIELD(id);
54  reg_val = reg_val << 32;
55  reg_val |= low;
56  
57  wrmsrl(HV_X64_MSR_ICR, reg_val);
58  }
59  
60  static u32 hv_apic_read(u32 reg)
61  {
62  u32 reg_val, hi;
63  
64  switch (reg) {
65  case APIC_EOI:
66  rdmsr(HV_X64_MSR_EOI, reg_val, hi);
67  return reg_val;
68  case APIC_TASKPRI:
69  rdmsr(HV_X64_MSR_TPR, reg_val, hi);
70  return reg_val;
71  
72  default:
73  return native_apic_mem_read(reg);
74  }
75  }
76  
77  static void hv_apic_write(u32 reg, u32 val)
78  {
79  switch (reg) {
80  case APIC_EOI:
81  wrmsr(HV_X64_MSR_EOI, val, 0);
82  break;
83  case APIC_TASKPRI:
84  wrmsr(HV_X64_MSR_TPR, val, 0);
85  break;
86  default:
87  native_apic_mem_write(reg, val);
88  }
89  }
90  
91  static void hv_apic_eoi_write(u32 reg, u32 val)
92  {
93  wrmsr(HV_X64_MSR_EOI, val, 0);
94  }
95  
96  /*
97   * IPI implementation on Hyper-V.
98   */
99  
   100  static int __send_ipi_mask(const struct cpumask *mask, int vector)
   101  {
   102  int

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread kbuild test robot

Hi Srinivasan,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.17-rc2]
[also build test ERROR on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
config: i386-randconfig-s1-04271426 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_read':
   arch/x86/hyperv/hv_apic.c:73:10: error: implicit declaration of function 
'native_apic_mem_read' [-Werror=implicit-function-declaration]
  return native_apic_mem_read(reg);
 ^~~~
   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_write':
   arch/x86/hyperv/hv_apic.c:87:3: error: implicit declaration of function 
'native_apic_mem_write' [-Werror=implicit-function-declaration]
  native_apic_mem_write(reg, val);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi':
   arch/x86/hyperv/hv_apic.c:155:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI(cpu, vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_mask':
   arch/x86/hyperv/hv_apic.c:161:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_mask(mask, vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_mask_allbutself':
   arch/x86/hyperv/hv_apic.c:174:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_mask_allbutself(mask, vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_all':
   arch/x86/hyperv/hv_apic.c:185:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_all(vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_self':
   arch/x86/hyperv/hv_apic.c:191:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_self(vector);
  ^
   arch/x86/hyperv/hv_apic.c: In function 'hv_apic_init':
   arch/x86/hyperv/hv_apic.c:204:16: error: 'apic' undeclared (first use in 
this function)
  orig_apic = *apic;
   ^~~~
   arch/x86/hyperv/hv_apic.c:204:16: note: each undeclared identifier is 
reported only once for each function it appears in
>> arch/x86/hyperv/hv_apic.c:204:3: error: 'orig_apic' has an incomplete type 
>> 'struct apic'
  orig_apic = *apic;
  ^
   arch/x86/hyperv/hv_apic.c:217:3: error: implicit declaration of function 
'apic_set_eoi_write' [-Werror=implicit-function-declaration]
  apic_set_eoi_write(hv_apic_eoi_write);
  ^~
   arch/x86/hyperv/hv_apic.c: At top level:
>> arch/x86/hyperv/hv_apic.c:39:20: error: storage size of 'orig_apic' isn't 
>> known
static struct apic orig_apic;
   ^
   cc1: some warnings being treated as errors

vim +204 arch/x86/hyperv/hv_apic.c

38  
  > 39  static struct apic orig_apic;
40  
41  static u64 hv_apic_icr_read(void)
42  {
43  u64 reg_val;
44  
45  rdmsrl(HV_X64_MSR_ICR, reg_val);
46  return reg_val;
47  }
48  
49  static void hv_apic_icr_write(u32 low, u32 id)
50  {
51  u64 reg_val;
52  
53  reg_val = SET_APIC_DEST_FIELD(id);
54  reg_val = reg_val << 32;
55  reg_val |= low;
56  
57  wrmsrl(HV_X64_MSR_ICR, reg_val);
58  }
59  
60  static u32 hv_apic_read(u32 reg)
61  {
62  u32 reg_val, hi;
63  
64  switch (reg) {
65  case APIC_EOI:
66  rdmsr(HV_X64_MSR_EOI, reg_val, hi);
67  return reg_val;
68  case APIC_TASKPRI:
69  rdmsr(HV_X64_MSR_TPR, reg_val, hi);
70  return reg_val;
71  
72  default:
73  return native_apic_mem_read(reg);
74  }
75  }
76  
77  static void hv_apic_write(u32 reg, u32 val)
78  {
79  switch (reg) {
80  case APIC_EOI:
81  wrmsr(HV_X64_MSR_EOI, val, 0);
82  break;
83  case APIC_TASKPRI:
84  wrmsr(HV_X64_MSR_TPR, val, 0);
85  break;
86  default:
87  native_apic_mem_write(reg, val);
88  }
89  }
90  
91  static void hv_apic_eoi_write(u32 reg, u32 val)
92  {
93  wrmsr(HV_X64_MSR_EOI, val, 0);
94  }
95  
96  /*
97   * IPI implementation on Hyper-V.
98   */
99  
   100  static int __send_ipi_mask(const struct cpumask *mask, int vector)
   101  {
   102  int

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread kbuild test robot

Hi Srinivasan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v4.17-rc2]
[also build test WARNING on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> arch/x86/hyperv/hv_init.c:105:30: sparse: incorrect type in initializer 
>> (different address spaces) @@expected void const [noderef] 
>> *__vpp_verify @@got const [noderef] *__vpp_verify @@
   arch/x86/hyperv/hv_init.c:105:30:expected void const [noderef] 
*__vpp_verify
   arch/x86/hyperv/hv_init.c:105:30:got void [noderef] **
   arch/x86/hyperv/hv_init.c:229:30: sparse: incorrect type in initializer 
(different address spaces) @@expected void const [noderef] 
*__vpp_verify @@got const [noderef] *__vpp_verify @@
   arch/x86/hyperv/hv_init.c:229:30:expected void const [noderef] 
*__vpp_verify
   arch/x86/hyperv/hv_init.c:229:30:got void [noderef] **
>> arch/x86/hyperv/hv_init.c:275:31: sparse: incorrect type in assignment 
>> (different address spaces) @@expected void [noderef] **extern 
>> [addressable] [toplevel] hyperv_pcpu_input_arg @@got addressable] 
>> [toplevel] hyperv_pcpu_input_arg @@
   arch/x86/hyperv/hv_init.c:275:31:expected void [noderef] **extern 
[addressable] [toplevel] hyperv_pcpu_input_arg
   arch/x86/hyperv/hv_init.c:275:31:got void *[noderef] *
   arch/x86/include/asm/paravirt.h:150:9: sparse: cast truncates bits from 
constant value (8000 becomes 0)
--
>> arch/x86/hyperv/hv_apic.c:118:41: sparse: incorrect type in initializer 
>> (different address spaces) @@expected void const [noderef] 
>> *__vpp_verify @@got const [noderef] *__vpp_verify @@
   arch/x86/hyperv/hv_apic.c:118:41:expected void const [noderef] 
*__vpp_verify
   arch/x86/hyperv/hv_apic.c:118:41:got void [noderef] **

vim +105 arch/x86/hyperv/hv_init.c

98  
99  static int hv_cpu_init(unsigned int cpu)
   100  {
   101  u64 msr_vp_index;
   102  struct hv_vp_assist_page **hvp = 
_vp_assist_page[smp_processor_id()];
   103  void **input_arg;
   104  
 > 105  input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
   106  *input_arg = page_address(alloc_page(GFP_ATOMIC));
   107  
   108  hv_get_vp_index(msr_vp_index);
   109  
   110  hv_vp_index[smp_processor_id()] = msr_vp_index;
   111  
   112  if (msr_vp_index > hv_max_vp_index)
   113  hv_max_vp_index = msr_vp_index;
   114  
   115  if (!hv_vp_assist_page)
   116  return 0;
   117  
   118  if (!*hvp)
   119  *hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
   120  
   121  if (*hvp) {
   122  u64 val;
   123  
   124  val = vmalloc_to_pfn(*hvp);
   125  val = (val << HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT) |
   126  HV_X64_MSR_VP_ASSIST_PAGE_ENABLE;
   127  
   128  wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, val);
   129  }
   130  
   131  return 0;
   132  }
   133  
   134  static void (*hv_reenlightenment_cb)(void);
   135  
   136  static void hv_reenlightenment_notify(struct work_struct *dummy)
   137  {
   138  struct hv_tsc_emulation_status emu_status;
   139  
   140  rdmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)_status);
   141  
   142  /* Don't issue the callback if TSC accesses are not emulated */
   143  if (hv_reenlightenment_cb && emu_status.inprogress)
   144  hv_reenlightenment_cb();
   145  }
   146  static DECLARE_DELAYED_WORK(hv_reenlightenment_work, 
hv_reenlightenment_notify);
   147  
   148  void hyperv_stop_tsc_emulation(void)
   149  {
   150  u64 freq;
   151  struct hv_tsc_emulation_status emu_status;
   152  
   153  rdmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)_status);
   154  emu_status.inprogress = 0;
   155  wrmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)_status);
   156  
   157  rdmsrl(HV_X64_MSR_TSC_FREQUENCY, freq);
   158  tsc_khz = div64_u64(freq, 1000);
   159  }
   160  EXPORT_SYMBOL_GPL(hyperv_stop_tsc_emulation);
   161  
   162  static inline bool hv_reenlightenment_available(void)
   163  {
   164  /*
   165   * Check for required features and priviliges to make TSC 
frequency
   166   * change notifications work.
   167   */
   168  return ms_hyperv.features & HV_X64_ACCESS_FREQUENCY_MSRS &&
   169  ms_hyperv.misc_features & 
HV_FEATURE_FREQUENCY_MSRS_AVAILABLE &&
   170

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread kbuild test robot

Hi Srinivasan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v4.17-rc2]
[also build test WARNING on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> arch/x86/hyperv/hv_init.c:105:30: sparse: incorrect type in initializer 
>> (different address spaces) @@expected void const [noderef] 
>> *__vpp_verify @@got const [noderef] *__vpp_verify @@
   arch/x86/hyperv/hv_init.c:105:30:expected void const [noderef] 
*__vpp_verify
   arch/x86/hyperv/hv_init.c:105:30:got void [noderef] **
   arch/x86/hyperv/hv_init.c:229:30: sparse: incorrect type in initializer 
(different address spaces) @@expected void const [noderef] 
*__vpp_verify @@got const [noderef] *__vpp_verify @@
   arch/x86/hyperv/hv_init.c:229:30:expected void const [noderef] 
*__vpp_verify
   arch/x86/hyperv/hv_init.c:229:30:got void [noderef] **
>> arch/x86/hyperv/hv_init.c:275:31: sparse: incorrect type in assignment 
>> (different address spaces) @@expected void [noderef] **extern 
>> [addressable] [toplevel] hyperv_pcpu_input_arg @@got addressable] 
>> [toplevel] hyperv_pcpu_input_arg @@
   arch/x86/hyperv/hv_init.c:275:31:expected void [noderef] **extern 
[addressable] [toplevel] hyperv_pcpu_input_arg
   arch/x86/hyperv/hv_init.c:275:31:got void *[noderef] *
   arch/x86/include/asm/paravirt.h:150:9: sparse: cast truncates bits from 
constant value (8000 becomes 0)
--
>> arch/x86/hyperv/hv_apic.c:118:41: sparse: incorrect type in initializer 
>> (different address spaces) @@expected void const [noderef] 
>> *__vpp_verify @@got const [noderef] *__vpp_verify @@
   arch/x86/hyperv/hv_apic.c:118:41:expected void const [noderef] 
*__vpp_verify
   arch/x86/hyperv/hv_apic.c:118:41:got void [noderef] **

vim +105 arch/x86/hyperv/hv_init.c

98  
99  static int hv_cpu_init(unsigned int cpu)
   100  {
   101  u64 msr_vp_index;
   102  struct hv_vp_assist_page **hvp = 
_vp_assist_page[smp_processor_id()];
   103  void **input_arg;
   104  
 > 105  input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
   106  *input_arg = page_address(alloc_page(GFP_ATOMIC));
   107  
   108  hv_get_vp_index(msr_vp_index);
   109  
   110  hv_vp_index[smp_processor_id()] = msr_vp_index;
   111  
   112  if (msr_vp_index > hv_max_vp_index)
   113  hv_max_vp_index = msr_vp_index;
   114  
   115  if (!hv_vp_assist_page)
   116  return 0;
   117  
   118  if (!*hvp)
   119  *hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL);
   120  
   121  if (*hvp) {
   122  u64 val;
   123  
   124  val = vmalloc_to_pfn(*hvp);
   125  val = (val << HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT) |
   126  HV_X64_MSR_VP_ASSIST_PAGE_ENABLE;
   127  
   128  wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, val);
   129  }
   130  
   131  return 0;
   132  }
   133  
   134  static void (*hv_reenlightenment_cb)(void);
   135  
   136  static void hv_reenlightenment_notify(struct work_struct *dummy)
   137  {
   138  struct hv_tsc_emulation_status emu_status;
   139  
   140  rdmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)_status);
   141  
   142  /* Don't issue the callback if TSC accesses are not emulated */
   143  if (hv_reenlightenment_cb && emu_status.inprogress)
   144  hv_reenlightenment_cb();
   145  }
   146  static DECLARE_DELAYED_WORK(hv_reenlightenment_work, 
hv_reenlightenment_notify);
   147  
   148  void hyperv_stop_tsc_emulation(void)
   149  {
   150  u64 freq;
   151  struct hv_tsc_emulation_status emu_status;
   152  
   153  rdmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)_status);
   154  emu_status.inprogress = 0;
   155  wrmsrl(HV_X64_MSR_TSC_EMULATION_STATUS, *(u64 *)_status);
   156  
   157  rdmsrl(HV_X64_MSR_TSC_FREQUENCY, freq);
   158  tsc_khz = div64_u64(freq, 1000);
   159  }
   160  EXPORT_SYMBOL_GPL(hyperv_stop_tsc_emulation);
   161  
   162  static inline bool hv_reenlightenment_available(void)
   163  {
   164  /*
   165   * Check for required features and priviliges to make TSC 
frequency
   166   * change notifications work.
   167   */
   168  return ms_hyperv.features & HV_X64_ACCESS_FREQUENCY_MSRS &&
   169  ms_hyperv.misc_features & 
HV_FEATURE_FREQUENCY_MSRS_AVAILABLE &&
   170

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread KY Srinivasan



> -Original Message-
> From: Dan Carpenter <dan.carpen...@oracle.com>
> Sent: Thursday, April 26, 2018 2:32 PM
> To: KY Srinivasan <k...@microsoft.com>
> Cc: x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; t...@linutronix.de;
> h...@zytor.com; Stephen Hemminger <sthem...@microsoft.com>; Michael
> Kelley (EOSG) <michael.h.kel...@microsoft.com>; vkuzn...@redhat.com
> Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> On Wed, Apr 25, 2018 at 11:12:47AM -0700, k...@linuxonhyperv.com wrote:
> > +/*
> > + * IPI implementation on Hyper-V.
> > + */
> > +
> > +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> > +{
> > +   int cur_cpu, vcpu;
> > +   struct ipi_arg_non_ex **arg;
> > +   struct ipi_arg_non_ex *ipi_arg;
> > +   int ret = 1;
> 
> Not specifically related to this patch, but hv code sometimes returns 1
> on error or U64_MAX.  It's slightly magical.  Maybe
> HV_STATUS_INVALID_HYPERCALL_INPUT (3) would be more appropriate?
> Or we
> could make a new more generic error code:
> 
> #define HV_STATUS_INVALID1

Good point. We will look at cleaning this up.

Regards,

K. Y

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread KY Srinivasan



> -Original Message-
> From: Dan Carpenter 
> Sent: Thursday, April 26, 2018 2:32 PM
> To: KY Srinivasan 
> Cc: x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; t...@linutronix.de;
> h...@zytor.com; Stephen Hemminger ; Michael
> Kelley (EOSG) ; vkuzn...@redhat.com
> Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> On Wed, Apr 25, 2018 at 11:12:47AM -0700, k...@linuxonhyperv.com wrote:
> > +/*
> > + * IPI implementation on Hyper-V.
> > + */
> > +
> > +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> > +{
> > +   int cur_cpu, vcpu;
> > +   struct ipi_arg_non_ex **arg;
> > +   struct ipi_arg_non_ex *ipi_arg;
> > +   int ret = 1;
> 
> Not specifically related to this patch, but hv code sometimes returns 1
> on error or U64_MAX.  It's slightly magical.  Maybe
> HV_STATUS_INVALID_HYPERCALL_INPUT (3) would be more appropriate?
> Or we
> could make a new more generic error code:
> 
> #define HV_STATUS_INVALID1

Good point. We will look at cleaning this up.

Regards,

K. Y

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread KY Srinivasan



> -Original Message-
> From: Michael Kelley (EOSG)
> Sent: Thursday, April 26, 2018 3:55 PM
> To: KY Srinivasan <k...@microsoft.com>; x...@kernel.org;
> gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; t...@linutronix.de; h...@zytor.com; Stephen
> Hemminger <sthem...@microsoft.com>; vkuzn...@redhat.com
> Subject: RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> > -Original Message-
> > From: k...@linuxonhyperv.com <k...@linuxonhyperv.com>
> > Sent: Wednesday, April 25, 2018 11:13 AM
> > To: x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org;
> > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com;
> > t...@linutronix.de; h...@zytor.com; Stephen Hemminger
> <sthem...@microsoft.com>;
> > Michael Kelley (EOSG) <michael.h.kel...@microsoft.com>;
> vkuzn...@redhat.com
> > Cc: KY Srinivasan <k...@microsoft.com>
> > Subject: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> >
> > From: "K. Y. Srinivasan" <k...@microsoft.com>
> >
> > Hyper-V supports hypercalls to implement IPI; use them.
> >
> > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > ---
> >  arch/x86/hyperv/hv_apic.c  | 125
> +
> >  arch/x86/hyperv/hv_init.c  |  17 +
> >  arch/x86/include/asm/hyperv-tlfs.h |   9 +++
> >  arch/x86/include/asm/mshyperv.h|   1 +
> >  4 files changed, 152 insertions(+)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index e0a5b36208fc..7f3322ecfb01 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -30,6 +30,14 @@
> >  #include 
> >  #include 
> >
> > +struct ipi_arg_non_ex {
> > +   u32 vector;
> > +   u32 reserved;
> > +   u64 cpu_mask;
> > +};
> 
> I think we'd like to put structures like this, which are defined in the
> Hyper-V Top Level Functional Spec, in hyperv-tlfs.h.  Also, the 5.0b
> version of the TLFS, which is latest, shows this structure on page 100:
> 
>   u32 vector;
>   u8  targetvtl;
>   u8  reserved[3];
>   u64 cpu_mask;
> 

Good point. I will make the necessary adjustments.

Regards,

K. Y

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread KY Srinivasan



> -Original Message-
> From: Michael Kelley (EOSG)
> Sent: Thursday, April 26, 2018 3:55 PM
> To: KY Srinivasan ; x...@kernel.org;
> gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com; t...@linutronix.de; h...@zytor.com; Stephen
> Hemminger ; vkuzn...@redhat.com
> Subject: RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> > -Original Message-
> > From: k...@linuxonhyperv.com 
> > Sent: Wednesday, April 25, 2018 11:13 AM
> > To: x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org;
> > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com;
> jasow...@redhat.com;
> > t...@linutronix.de; h...@zytor.com; Stephen Hemminger
> ;
> > Michael Kelley (EOSG) ;
> vkuzn...@redhat.com
> > Cc: KY Srinivasan 
> > Subject: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> >
> > From: "K. Y. Srinivasan" 
> >
> > Hyper-V supports hypercalls to implement IPI; use them.
> >
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> >  arch/x86/hyperv/hv_apic.c  | 125
> +
> >  arch/x86/hyperv/hv_init.c  |  17 +
> >  arch/x86/include/asm/hyperv-tlfs.h |   9 +++
> >  arch/x86/include/asm/mshyperv.h|   1 +
> >  4 files changed, 152 insertions(+)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index e0a5b36208fc..7f3322ecfb01 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -30,6 +30,14 @@
> >  #include 
> >  #include 
> >
> > +struct ipi_arg_non_ex {
> > +   u32 vector;
> > +   u32 reserved;
> > +   u64 cpu_mask;
> > +};
> 
> I think we'd like to put structures like this, which are defined in the
> Hyper-V Top Level Functional Spec, in hyperv-tlfs.h.  Also, the 5.0b
> version of the TLFS, which is latest, shows this structure on page 100:
> 
>   u32 vector;
>   u8  targetvtl;
>   u8  reserved[3];
>   u64 cpu_mask;
> 

Good point. I will make the necessary adjustments.

Regards,

K. Y

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread kbuild test robot

Hi Srinivasan,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.17-rc2]
[also build test ERROR on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
config: i386-randconfig-b0-04270034 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_read':
   arch/x86//hyperv/hv_apic.c:73:3: error: implicit declaration of function 
'native_apic_mem_read' [-Werror=implicit-function-declaration]
  return native_apic_mem_read(reg);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_write':
   arch/x86//hyperv/hv_apic.c:87:3: error: implicit declaration of function 
'native_apic_mem_write' [-Werror=implicit-function-declaration]
  native_apic_mem_write(reg, val);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi':
>> arch/x86//hyperv/hv_apic.c:155:3: error: invalid use of undefined type 
>> 'struct apic'
  orig_apic.send_IPI(cpu, vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_mask':
   arch/x86//hyperv/hv_apic.c:161:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_mask(mask, vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_mask_allbutself':
   arch/x86//hyperv/hv_apic.c:174:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_mask_allbutself(mask, vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_all':
   arch/x86//hyperv/hv_apic.c:185:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_all(vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_self':
   arch/x86//hyperv/hv_apic.c:191:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_self(vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_init':
   arch/x86//hyperv/hv_apic.c:204:16: error: 'apic' undeclared (first use in 
this function)
  orig_apic = *apic;
   ^
   arch/x86//hyperv/hv_apic.c:204:16: note: each undeclared identifier is 
reported only once for each function it appears in
>> arch/x86//hyperv/hv_apic.c:204:3: error: 'orig_apic' has an incomplete type
  orig_apic = *apic;
  ^
   arch/x86//hyperv/hv_apic.c:217:3: error: implicit declaration of function 
'apic_set_eoi_write' [-Werror=implicit-function-declaration]
  apic_set_eoi_write(hv_apic_eoi_write);
  ^
   cc1: some warnings being treated as errors

vim +/orig_apic +204 arch/x86//hyperv/hv_apic.c

59  
60  static u32 hv_apic_read(u32 reg)
61  {
62  u32 reg_val, hi;
63  
64  switch (reg) {
65  case APIC_EOI:
66  rdmsr(HV_X64_MSR_EOI, reg_val, hi);
67  return reg_val;
68  case APIC_TASKPRI:
69  rdmsr(HV_X64_MSR_TPR, reg_val, hi);
70  return reg_val;
71  
72  default:
  > 73  return native_apic_mem_read(reg);
74  }
75  }
76  
77  static void hv_apic_write(u32 reg, u32 val)
78  {
79  switch (reg) {
80  case APIC_EOI:
81  wrmsr(HV_X64_MSR_EOI, val, 0);
82  break;
83  case APIC_TASKPRI:
84  wrmsr(HV_X64_MSR_TPR, val, 0);
85  break;
86  default:
87  native_apic_mem_write(reg, val);
88  }
89  }
90  
91  static void hv_apic_eoi_write(u32 reg, u32 val)
92  {
93  wrmsr(HV_X64_MSR_EOI, val, 0);
94  }
95  
96  /*
97   * IPI implementation on Hyper-V.
98   */
99  
   100  static int __send_ipi_mask(const struct cpumask *mask, int vector)
   101  {
   102  int cur_cpu, vcpu;
   103  struct ipi_arg_non_ex **arg;
   104  struct ipi_arg_non_ex *ipi_arg;
   105  int ret = 1;
   106  unsigned long flags;
   107  
   108  if (cpumask_empty(mask))
   109  return 0;
   110  
   111  if (!hv_hypercall_pg)
   112  return ret;
   113  
   114  if ((vector < HV_IPI_LOW_VECTOR) || (vector > 
HV_IPI_HIGH_VECTOR))
   115  return ret;
   116  
   117  local_irq_save(flags);
   118  arg = (struct ipi_arg_non_ex 
**)this_cpu_ptr(hyperv_pcpu_input_arg);
   119  
   120  ipi_arg = *arg;
   121  if (unlikely(!ipi_arg))
   122  goto ipi_mask_done;
   123  
   124  
   125  ipi_arg->vector = vector;
   126  ipi_arg->reserved = 0;
   127

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread kbuild test robot

Hi Srinivasan,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.17-rc2]
[also build test ERROR on next-20180426]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180427-114416
config: i386-randconfig-b0-04270034 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_read':
   arch/x86//hyperv/hv_apic.c:73:3: error: implicit declaration of function 
'native_apic_mem_read' [-Werror=implicit-function-declaration]
  return native_apic_mem_read(reg);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_write':
   arch/x86//hyperv/hv_apic.c:87:3: error: implicit declaration of function 
'native_apic_mem_write' [-Werror=implicit-function-declaration]
  native_apic_mem_write(reg, val);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi':
>> arch/x86//hyperv/hv_apic.c:155:3: error: invalid use of undefined type 
>> 'struct apic'
  orig_apic.send_IPI(cpu, vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_mask':
   arch/x86//hyperv/hv_apic.c:161:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_mask(mask, vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_mask_allbutself':
   arch/x86//hyperv/hv_apic.c:174:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_mask_allbutself(mask, vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_all':
   arch/x86//hyperv/hv_apic.c:185:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_all(vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_send_ipi_self':
   arch/x86//hyperv/hv_apic.c:191:3: error: invalid use of undefined type 
'struct apic'
  orig_apic.send_IPI_self(vector);
  ^
   arch/x86//hyperv/hv_apic.c: In function 'hv_apic_init':
   arch/x86//hyperv/hv_apic.c:204:16: error: 'apic' undeclared (first use in 
this function)
  orig_apic = *apic;
   ^
   arch/x86//hyperv/hv_apic.c:204:16: note: each undeclared identifier is 
reported only once for each function it appears in
>> arch/x86//hyperv/hv_apic.c:204:3: error: 'orig_apic' has an incomplete type
  orig_apic = *apic;
  ^
   arch/x86//hyperv/hv_apic.c:217:3: error: implicit declaration of function 
'apic_set_eoi_write' [-Werror=implicit-function-declaration]
  apic_set_eoi_write(hv_apic_eoi_write);
  ^
   cc1: some warnings being treated as errors

vim +/orig_apic +204 arch/x86//hyperv/hv_apic.c

59  
60  static u32 hv_apic_read(u32 reg)
61  {
62  u32 reg_val, hi;
63  
64  switch (reg) {
65  case APIC_EOI:
66  rdmsr(HV_X64_MSR_EOI, reg_val, hi);
67  return reg_val;
68  case APIC_TASKPRI:
69  rdmsr(HV_X64_MSR_TPR, reg_val, hi);
70  return reg_val;
71  
72  default:
  > 73  return native_apic_mem_read(reg);
74  }
75  }
76  
77  static void hv_apic_write(u32 reg, u32 val)
78  {
79  switch (reg) {
80  case APIC_EOI:
81  wrmsr(HV_X64_MSR_EOI, val, 0);
82  break;
83  case APIC_TASKPRI:
84  wrmsr(HV_X64_MSR_TPR, val, 0);
85  break;
86  default:
87  native_apic_mem_write(reg, val);
88  }
89  }
90  
91  static void hv_apic_eoi_write(u32 reg, u32 val)
92  {
93  wrmsr(HV_X64_MSR_EOI, val, 0);
94  }
95  
96  /*
97   * IPI implementation on Hyper-V.
98   */
99  
   100  static int __send_ipi_mask(const struct cpumask *mask, int vector)
   101  {
   102  int cur_cpu, vcpu;
   103  struct ipi_arg_non_ex **arg;
   104  struct ipi_arg_non_ex *ipi_arg;
   105  int ret = 1;
   106  unsigned long flags;
   107  
   108  if (cpumask_empty(mask))
   109  return 0;
   110  
   111  if (!hv_hypercall_pg)
   112  return ret;
   113  
   114  if ((vector < HV_IPI_LOW_VECTOR) || (vector > 
HV_IPI_HIGH_VECTOR))
   115  return ret;
   116  
   117  local_irq_save(flags);
   118  arg = (struct ipi_arg_non_ex 
**)this_cpu_ptr(hyperv_pcpu_input_arg);
   119  
   120  ipi_arg = *arg;
   121  if (unlikely(!ipi_arg))
   122  goto ipi_mask_done;
   123  
   124  
   125  ipi_arg->vector = vector;
   126  ipi_arg->reserved = 0;
   127

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread KY Srinivasan



> -Original Message-
> From: Thomas Gleixner <t...@linutronix.de>
> Sent: Thursday, April 26, 2018 3:09 PM
> To: KY Srinivasan <k...@microsoft.com>
> Cc: x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; h...@zytor.com; Stephen
> Hemminger <sthem...@microsoft.com>; Michael Kelley (EOSG)
> <michael.h.kel...@microsoft.com>; vkuzn...@redhat.com
> Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote:
> > +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> > +{
> > +   int cur_cpu, vcpu;
> > +   struct ipi_arg_non_ex **arg;
> > +   struct ipi_arg_non_ex *ipi_arg;
> > +   int ret = 1;
> 
> So this indicates whether __send_ipi_mask() can send to @mask or not. So
> please make it a bool and let it return false when it does not work, true
> otherwise. If you had used -E then it would have been more obvious,
> but
> this is really a boolean decision.

Agreed.
> 
> > +   unsigned long flags;
> > +
> > +   if (cpumask_empty(mask))
> > +   return 0;
> > +
> > +   if (!hv_hypercall_pg)
> > +   return ret;
> > +
> > +   if ((vector < HV_IPI_LOW_VECTOR) || (vector >
> HV_IPI_HIGH_VECTOR))
> > +   return ret;
> > +
> > +   local_irq_save(flags);
> > +   arg = (struct ipi_arg_non_ex
> **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +
> > +   ipi_arg = *arg;
> > +   if (unlikely(!ipi_arg))
> > +   goto ipi_mask_done;
> > +
> > +
> 
> Stray newline
> 
> > +   ipi_arg->vector = vector;
> > +   ipi_arg->reserved = 0;
> > +   ipi_arg->cpu_mask = 0;
> > +
> > +   for_each_cpu(cur_cpu, mask) {
> > +   vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> > +   if (vcpu >= 64)
> > +   goto ipi_mask_done;
> 
> This is completely magic and deserves a comment.
> 
> > +
> > +   __set_bit(vcpu, (unsigned long *)_arg->cpu_mask);
> > +   }
> > +
> > +   ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> > +
> > +ipi_mask_done:
> > +   local_irq_restore(flags);
> > +   return ret;
> > +}
> 
> 
> 
> >  static int hv_cpu_init(unsigned int cpu)
> >  {
> > u64 msr_vp_index;
> > struct hv_vp_assist_page **hvp =
> _vp_assist_page[smp_processor_id()];
> > +   void **input_arg;
> > +
> > +   input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +   *input_arg = page_address(alloc_page(GFP_ATOMIC));
> 
> This is called from the cpu hotplug thread and there is no need for an
> atomic allocation. Please use GFP_KERNEL.
> 
> > hv_get_vp_index(msr_vp_index);
> >
> > @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu)
> >  {
> > struct hv_reenlightenment_control re_ctrl;
> > unsigned int new_cpu;
> > +   void **input_arg;
> > +
> > +   input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +   free_page((unsigned long)*input_arg);
> 
> Hrm. Again this is called from the CPU hotplug thread when the cou is about
> to go down. But you can be scheduled out after free() and before disabling
> the assist thing below and the pointer persist. There is no guarantee that
> nothing sends an IPI anymore after this point.
> 
> So you have two options here:
> 
>   1) Disable interrupts, get the pointer, set the per cpu pointer to NULL,
>  reenable interruots and free the page
I will implement this approach.
> 
>   2) Keep the page around and check for it in the CPU UP path and avoid the
>  allocation when the CPU comes online again.
> 
> > if (hv_vp_assist_page && hv_vp_assist_page[cpu])
> > wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> > @@ -260,6 +271,12 @@ void __init hyperv_init(void)
> > if ((ms_hyperv.features & required_msrs) != required_msrs)
> > return;
> >
> > +   /* Allocate the per-CPU state for the hypercall input arg */
> > +   hyperv_pcpu_input_arg = alloc_percpu(void  *);
> > +
> > +   if (hyperv_pcpu_input_arg == NULL)
> > +   return;
> 
> Huch. When that allocation fails, you return and ignore the rest of the
> function which has been there before. Weird decision.
I should have explained this. Failure of this allocation means that we would 
not have the
per-cpu hypercall input page which in turn would mean that we would not be able 
to invoke
any hypercalls. 

Regards,

K. Y

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-27 Thread KY Srinivasan



> -Original Message-
> From: Thomas Gleixner 
> Sent: Thursday, April 26, 2018 3:09 PM
> To: KY Srinivasan 
> Cc: x...@kernel.org; gre...@linuxfoundation.org; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de;
> a...@canonical.com; jasow...@redhat.com; h...@zytor.com; Stephen
> Hemminger ; Michael Kelley (EOSG)
> ; vkuzn...@redhat.com
> Subject: Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote:
> > +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> > +{
> > +   int cur_cpu, vcpu;
> > +   struct ipi_arg_non_ex **arg;
> > +   struct ipi_arg_non_ex *ipi_arg;
> > +   int ret = 1;
> 
> So this indicates whether __send_ipi_mask() can send to @mask or not. So
> please make it a bool and let it return false when it does not work, true
> otherwise. If you had used -E then it would have been more obvious,
> but
> this is really a boolean decision.

Agreed.
> 
> > +   unsigned long flags;
> > +
> > +   if (cpumask_empty(mask))
> > +   return 0;
> > +
> > +   if (!hv_hypercall_pg)
> > +   return ret;
> > +
> > +   if ((vector < HV_IPI_LOW_VECTOR) || (vector >
> HV_IPI_HIGH_VECTOR))
> > +   return ret;
> > +
> > +   local_irq_save(flags);
> > +   arg = (struct ipi_arg_non_ex
> **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +
> > +   ipi_arg = *arg;
> > +   if (unlikely(!ipi_arg))
> > +   goto ipi_mask_done;
> > +
> > +
> 
> Stray newline
> 
> > +   ipi_arg->vector = vector;
> > +   ipi_arg->reserved = 0;
> > +   ipi_arg->cpu_mask = 0;
> > +
> > +   for_each_cpu(cur_cpu, mask) {
> > +   vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> > +   if (vcpu >= 64)
> > +   goto ipi_mask_done;
> 
> This is completely magic and deserves a comment.
> 
> > +
> > +   __set_bit(vcpu, (unsigned long *)_arg->cpu_mask);
> > +   }
> > +
> > +   ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> > +
> > +ipi_mask_done:
> > +   local_irq_restore(flags);
> > +   return ret;
> > +}
> 
> 
> 
> >  static int hv_cpu_init(unsigned int cpu)
> >  {
> > u64 msr_vp_index;
> > struct hv_vp_assist_page **hvp =
> _vp_assist_page[smp_processor_id()];
> > +   void **input_arg;
> > +
> > +   input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +   *input_arg = page_address(alloc_page(GFP_ATOMIC));
> 
> This is called from the cpu hotplug thread and there is no need for an
> atomic allocation. Please use GFP_KERNEL.
> 
> > hv_get_vp_index(msr_vp_index);
> >
> > @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu)
> >  {
> > struct hv_reenlightenment_control re_ctrl;
> > unsigned int new_cpu;
> > +   void **input_arg;
> > +
> > +   input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > +   free_page((unsigned long)*input_arg);
> 
> Hrm. Again this is called from the CPU hotplug thread when the cou is about
> to go down. But you can be scheduled out after free() and before disabling
> the assist thing below and the pointer persist. There is no guarantee that
> nothing sends an IPI anymore after this point.
> 
> So you have two options here:
> 
>   1) Disable interrupts, get the pointer, set the per cpu pointer to NULL,
>  reenable interruots and free the page
I will implement this approach.
> 
>   2) Keep the page around and check for it in the CPU UP path and avoid the
>  allocation when the CPU comes online again.
> 
> > if (hv_vp_assist_page && hv_vp_assist_page[cpu])
> > wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> > @@ -260,6 +271,12 @@ void __init hyperv_init(void)
> > if ((ms_hyperv.features & required_msrs) != required_msrs)
> > return;
> >
> > +   /* Allocate the per-CPU state for the hypercall input arg */
> > +   hyperv_pcpu_input_arg = alloc_percpu(void  *);
> > +
> > +   if (hyperv_pcpu_input_arg == NULL)
> > +   return;
> 
> Huch. When that allocation fails, you return and ignore the rest of the
> function which has been there before. Weird decision.
I should have explained this. Failure of this allocation means that we would 
not have the
per-cpu hypercall input page which in turn would mean that we would not be able 
to invoke
any hypercalls. 

Regards,

K. Y

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-26 Thread Michael Kelley (EOSG)

> -Original Message-
> From: k...@linuxonhyperv.com 
> Sent: Wednesday, April 25, 2018 11:13 AM
> To: x...@kernel.org; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; 
> jasow...@redhat.com;
> t...@linutronix.de; h...@zytor.com; Stephen Hemminger 
> ;
> Michael Kelley (EOSG) ; vkuzn...@redhat.com
> Cc: KY Srinivasan 
> Subject: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> From: "K. Y. Srinivasan" 
> 
> Hyper-V supports hypercalls to implement IPI; use them.
> 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  arch/x86/hyperv/hv_apic.c  | 125 
> +
>  arch/x86/hyperv/hv_init.c  |  17 +
>  arch/x86/include/asm/hyperv-tlfs.h |   9 +++
>  arch/x86/include/asm/mshyperv.h|   1 +
>  4 files changed, 152 insertions(+)
> 
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index e0a5b36208fc..7f3322ecfb01 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -30,6 +30,14 @@
>  #include 
>  #include 
> 
> +struct ipi_arg_non_ex {
> + u32 vector;
> + u32 reserved;
> + u64 cpu_mask;
> +};

I think we'd like to put structures like this, which are defined in the
Hyper-V Top Level Functional Spec, in hyperv-tlfs.h.  Also, the 5.0b
version of the TLFS, which is latest, shows this structure on page 100:

u32 vector;
u8  targetvtl;
u8  reserved[3];
u64 cpu_mask;

> +
> +static struct apic orig_apic;
> +
>  static u64 hv_apic_icr_read(void)
>  {
>   u64 reg_val;
> @@ -85,8 +93,125 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
>   wrmsr(HV_X64_MSR_EOI, val, 0);
>  }
> 
> +/*
> + * IPI implementation on Hyper-V.
> + */
> +
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> + int cur_cpu, vcpu;
> + struct ipi_arg_non_ex **arg;
> + struct ipi_arg_non_ex *ipi_arg;
> + int ret = 1;
> + unsigned long flags;
> +
> + if (cpumask_empty(mask))
> + return 0;
> +
> + if (!hv_hypercall_pg)
> + return ret;
> +
> + if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
> + return ret;
> +
> + local_irq_save(flags);
> + arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> + ipi_arg = *arg;
> + if (unlikely(!ipi_arg))
> + goto ipi_mask_done;
> +
> +
> + ipi_arg->vector = vector;
> + ipi_arg->reserved = 0;
> + ipi_arg->cpu_mask = 0;
> +
> + for_each_cpu(cur_cpu, mask) {
> + vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> + if (vcpu >= 64)
> + goto ipi_mask_done;
> +
> + __set_bit(vcpu, (unsigned long *)_arg->cpu_mask);
> + }
> +
> + ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> +
> +ipi_mask_done:
> + local_irq_restore(flags);
> + return ret;
> +}
> +
> +static int __send_ipi_one(int cpu, int vector)
> +{
> + struct cpumask mask = CPU_MASK_NONE;
> +
> + cpumask_set_cpu(cpu, );
> + return __send_ipi_mask(, vector);
> +}
> +
> +static void hv_send_ipi(int cpu, int vector)
> +{
> + if (__send_ipi_one(cpu, vector))
> + orig_apic.send_IPI(cpu, vector);
> +}
> +
> +static void hv_send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> + if (__send_ipi_mask(mask, vector))
> + orig_apic.send_IPI_mask(mask, vector);
> +}
> +
> +static void hv_send_ipi_mask_allbutself(const struct cpumask *mask, int 
> vector)
> +{
> + unsigned int this_cpu = smp_processor_id();
> + struct cpumask new_mask;
> + const struct cpumask *local_mask;
> +
> + cpumask_copy(_mask, mask);
> + cpumask_clear_cpu(this_cpu, _mask);
> + local_mask = _mask;
> + if (__send_ipi_mask(local_mask, vector))
> + orig_apic.send_IPI_mask_allbutself(mask, vector);
> +}
> +
> +static void hv_send_ipi_allbutself(int vector)
> +{
> + hv_send_ipi_mask_allbutself(cpu_online_mask, vector);
> +}
> +
> +static void hv_send_ipi_all(int vector)
> +{
> + if (__send_ipi_mask(cpu_online_mask, vector))
> + orig_apic.send_IPI_all(vector);
> +}
> +
> +static void hv_send_ipi_self(int vector)
> +{
> + if (__send_ipi_one(smp_processor_id(), vector))
> + orig_apic.send_IPI_self(vector);
> +}
> +
>  void __init hv_apic_init(void)
>  {
> + if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
> + if (hyperv_pcpu_input_arg == NULL)
> + goto msr_based_access;
> +
> + pr_info("Hyper-V: Using IPI hypercalls\n");
> + /*
> +  * Set the IPI entry points.
> +  */
> + orig_apic = *apic;
> +
> + apic->send_IPI = hv_send_ipi;
> + apic->send_IPI_mask

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-26 Thread Michael Kelley (EOSG)

> -Original Message-
> From: k...@linuxonhyperv.com 
> Sent: Wednesday, April 25, 2018 11:13 AM
> To: x...@kernel.org; gre...@linuxfoundation.org; linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; 
> jasow...@redhat.com;
> t...@linutronix.de; h...@zytor.com; Stephen Hemminger 
> ;
> Michael Kelley (EOSG) ; vkuzn...@redhat.com
> Cc: KY Srinivasan 
> Subject: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments
> 
> From: "K. Y. Srinivasan" 
> 
> Hyper-V supports hypercalls to implement IPI; use them.
> 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  arch/x86/hyperv/hv_apic.c  | 125 
> +
>  arch/x86/hyperv/hv_init.c  |  17 +
>  arch/x86/include/asm/hyperv-tlfs.h |   9 +++
>  arch/x86/include/asm/mshyperv.h|   1 +
>  4 files changed, 152 insertions(+)
> 
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index e0a5b36208fc..7f3322ecfb01 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -30,6 +30,14 @@
>  #include 
>  #include 
> 
> +struct ipi_arg_non_ex {
> + u32 vector;
> + u32 reserved;
> + u64 cpu_mask;
> +};

I think we'd like to put structures like this, which are defined in the
Hyper-V Top Level Functional Spec, in hyperv-tlfs.h.  Also, the 5.0b
version of the TLFS, which is latest, shows this structure on page 100:

u32 vector;
u8  targetvtl;
u8  reserved[3];
u64 cpu_mask;

> +
> +static struct apic orig_apic;
> +
>  static u64 hv_apic_icr_read(void)
>  {
>   u64 reg_val;
> @@ -85,8 +93,125 @@ static void hv_apic_eoi_write(u32 reg, u32 val)
>   wrmsr(HV_X64_MSR_EOI, val, 0);
>  }
> 
> +/*
> + * IPI implementation on Hyper-V.
> + */
> +
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> + int cur_cpu, vcpu;
> + struct ipi_arg_non_ex **arg;
> + struct ipi_arg_non_ex *ipi_arg;
> + int ret = 1;
> + unsigned long flags;
> +
> + if (cpumask_empty(mask))
> + return 0;
> +
> + if (!hv_hypercall_pg)
> + return ret;
> +
> + if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
> + return ret;
> +
> + local_irq_save(flags);
> + arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> + ipi_arg = *arg;
> + if (unlikely(!ipi_arg))
> + goto ipi_mask_done;
> +
> +
> + ipi_arg->vector = vector;
> + ipi_arg->reserved = 0;
> + ipi_arg->cpu_mask = 0;
> +
> + for_each_cpu(cur_cpu, mask) {
> + vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> + if (vcpu >= 64)
> + goto ipi_mask_done;
> +
> + __set_bit(vcpu, (unsigned long *)_arg->cpu_mask);
> + }
> +
> + ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> +
> +ipi_mask_done:
> + local_irq_restore(flags);
> + return ret;
> +}
> +
> +static int __send_ipi_one(int cpu, int vector)
> +{
> + struct cpumask mask = CPU_MASK_NONE;
> +
> + cpumask_set_cpu(cpu, );
> + return __send_ipi_mask(, vector);
> +}
> +
> +static void hv_send_ipi(int cpu, int vector)
> +{
> + if (__send_ipi_one(cpu, vector))
> + orig_apic.send_IPI(cpu, vector);
> +}
> +
> +static void hv_send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> + if (__send_ipi_mask(mask, vector))
> + orig_apic.send_IPI_mask(mask, vector);
> +}
> +
> +static void hv_send_ipi_mask_allbutself(const struct cpumask *mask, int 
> vector)
> +{
> + unsigned int this_cpu = smp_processor_id();
> + struct cpumask new_mask;
> + const struct cpumask *local_mask;
> +
> + cpumask_copy(_mask, mask);
> + cpumask_clear_cpu(this_cpu, _mask);
> + local_mask = _mask;
> + if (__send_ipi_mask(local_mask, vector))
> + orig_apic.send_IPI_mask_allbutself(mask, vector);
> +}
> +
> +static void hv_send_ipi_allbutself(int vector)
> +{
> + hv_send_ipi_mask_allbutself(cpu_online_mask, vector);
> +}
> +
> +static void hv_send_ipi_all(int vector)
> +{
> + if (__send_ipi_mask(cpu_online_mask, vector))
> + orig_apic.send_IPI_all(vector);
> +}
> +
> +static void hv_send_ipi_self(int vector)
> +{
> + if (__send_ipi_one(smp_processor_id(), vector))
> + orig_apic.send_IPI_self(vector);
> +}
> +
>  void __init hv_apic_init(void)
>  {
> + if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
> + if (hyperv_pcpu_input_arg == NULL)
> + goto msr_based_access;
> +
> + pr_info("Hyper-V: Using IPI hypercalls\n");
> + /*
> +  * Set the IPI entry points.
> +  */
> + orig_apic = *apic;
> +
> + apic->send_IPI = hv_send_ipi;
> + apic->send_IPI_mask = hv_send_ipi_mask;
> + apic->send_IPI_mask_allbutself = hv_send_ipi_mask_allbutself;
> + apic->send_IPI_allbutself

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-26 Thread Thomas Gleixner

On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote:
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> + int cur_cpu, vcpu;
> + struct ipi_arg_non_ex **arg;
> + struct ipi_arg_non_ex *ipi_arg;
> + int ret = 1;

So this indicates whether __send_ipi_mask() can send to @mask or not. So
please make it a bool and let it return false when it does not work, true
otherwise. If you had used -E then it would have been more obvious, but
this is really a boolean decision.

> + unsigned long flags;
> +
> + if (cpumask_empty(mask))
> + return 0;
> +
> + if (!hv_hypercall_pg)
> + return ret;
> +
> + if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
> + return ret;
> +
> + local_irq_save(flags);
> + arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> + ipi_arg = *arg;
> + if (unlikely(!ipi_arg))
> + goto ipi_mask_done;
> +
> +

Stray newline

> + ipi_arg->vector = vector;
> + ipi_arg->reserved = 0;
> + ipi_arg->cpu_mask = 0;
> +
> + for_each_cpu(cur_cpu, mask) {
> + vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> + if (vcpu >= 64)
> + goto ipi_mask_done;

This is completely magic and deserves a comment.

> +
> + __set_bit(vcpu, (unsigned long *)_arg->cpu_mask);
> + }
> +
> + ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> +
> +ipi_mask_done:
> + local_irq_restore(flags);
> + return ret;
> +}

>  static int hv_cpu_init(unsigned int cpu)
>  {
>   u64 msr_vp_index;
>   struct hv_vp_assist_page **hvp = _vp_assist_page[smp_processor_id()];
> + void **input_arg;
> +
> + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> + *input_arg = page_address(alloc_page(GFP_ATOMIC));

This is called from the cpu hotplug thread and there is no need for an
atomic allocation. Please use GFP_KERNEL.

>   hv_get_vp_index(msr_vp_index);
>  
> @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu)
>  {
>   struct hv_reenlightenment_control re_ctrl;
>   unsigned int new_cpu;
> + void **input_arg;
> +
> + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> + free_page((unsigned long)*input_arg);

Hrm. Again this is called from the CPU hotplug thread when the cou is about
to go down. But you can be scheduled out after free() and before disabling
the assist thing below and the pointer persist. There is no guarantee that
nothing sends an IPI anymore after this point.

So you have two options here:

  1) Disable interrupts, get the pointer, set the per cpu pointer to NULL,
 reenable interruots and free the page

  2) Keep the page around and check for it in the CPU UP path and avoid the
 allocation when the CPU comes online again.

>   if (hv_vp_assist_page && hv_vp_assist_page[cpu])
>   wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> @@ -260,6 +271,12 @@ void __init hyperv_init(void)
>   if ((ms_hyperv.features & required_msrs) != required_msrs)
>   return;
>  
> + /* Allocate the per-CPU state for the hypercall input arg */
> + hyperv_pcpu_input_arg = alloc_percpu(void  *);
> +
> + if (hyperv_pcpu_input_arg == NULL)
> + return;

Huch. When that allocation fails, you return and ignore the rest of the
function which has been there before. Weird decision.

Thanks,

tglx

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-26 Thread Thomas Gleixner

On Wed, 25 Apr 2018, k...@linuxonhyperv.com wrote:
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> + int cur_cpu, vcpu;
> + struct ipi_arg_non_ex **arg;
> + struct ipi_arg_non_ex *ipi_arg;
> + int ret = 1;

So this indicates whether __send_ipi_mask() can send to @mask or not. So
please make it a bool and let it return false when it does not work, true
otherwise. If you had used -E then it would have been more obvious, but
this is really a boolean decision.

> + unsigned long flags;
> +
> + if (cpumask_empty(mask))
> + return 0;
> +
> + if (!hv_hypercall_pg)
> + return ret;
> +
> + if ((vector < HV_IPI_LOW_VECTOR) || (vector > HV_IPI_HIGH_VECTOR))
> + return ret;
> +
> + local_irq_save(flags);
> + arg = (struct ipi_arg_non_ex **)this_cpu_ptr(hyperv_pcpu_input_arg);
> +
> + ipi_arg = *arg;
> + if (unlikely(!ipi_arg))
> + goto ipi_mask_done;
> +
> +

Stray newline

> + ipi_arg->vector = vector;
> + ipi_arg->reserved = 0;
> + ipi_arg->cpu_mask = 0;
> +
> + for_each_cpu(cur_cpu, mask) {
> + vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> + if (vcpu >= 64)
> + goto ipi_mask_done;

This is completely magic and deserves a comment.

> +
> + __set_bit(vcpu, (unsigned long *)_arg->cpu_mask);
> + }
> +
> + ret = hv_do_hypercall(HVCALL_SEND_IPI, ipi_arg, NULL);
> +
> +ipi_mask_done:
> + local_irq_restore(flags);
> + return ret;
> +}

>  static int hv_cpu_init(unsigned int cpu)
>  {
>   u64 msr_vp_index;
>   struct hv_vp_assist_page **hvp = _vp_assist_page[smp_processor_id()];
> + void **input_arg;
> +
> + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> + *input_arg = page_address(alloc_page(GFP_ATOMIC));

This is called from the cpu hotplug thread and there is no need for an
atomic allocation. Please use GFP_KERNEL.

>   hv_get_vp_index(msr_vp_index);
>  
> @@ -217,6 +224,10 @@ static int hv_cpu_die(unsigned int cpu)
>  {
>   struct hv_reenlightenment_control re_ctrl;
>   unsigned int new_cpu;
> + void **input_arg;
> +
> + input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> + free_page((unsigned long)*input_arg);

Hrm. Again this is called from the CPU hotplug thread when the cou is about
to go down. But you can be scheduled out after free() and before disabling
the assist thing below and the pointer persist. There is no guarantee that
nothing sends an IPI anymore after this point.

So you have two options here:

  1) Disable interrupts, get the pointer, set the per cpu pointer to NULL,
 reenable interruots and free the page

  2) Keep the page around and check for it in the CPU UP path and avoid the
 allocation when the CPU comes online again.

>   if (hv_vp_assist_page && hv_vp_assist_page[cpu])
>   wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> @@ -260,6 +271,12 @@ void __init hyperv_init(void)
>   if ((ms_hyperv.features & required_msrs) != required_msrs)
>   return;
>  
> + /* Allocate the per-CPU state for the hypercall input arg */
> + hyperv_pcpu_input_arg = alloc_percpu(void  *);
> +
> + if (hyperv_pcpu_input_arg == NULL)
> + return;

Huch. When that allocation fails, you return and ignore the rest of the
function which has been there before. Weird decision.

Thanks,

tglx

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-26 Thread Dan Carpenter

On Wed, Apr 25, 2018 at 11:12:47AM -0700, k...@linuxonhyperv.com wrote:
> +/*
> + * IPI implementation on Hyper-V.
> + */
> +
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> + int cur_cpu, vcpu;
> + struct ipi_arg_non_ex **arg;
> + struct ipi_arg_non_ex *ipi_arg;
> + int ret = 1;

Not specifically related to this patch, but hv code sometimes returns 1
on error or U64_MAX.  It's slightly magical.  Maybe
HV_STATUS_INVALID_HYPERCALL_INPUT (3) would be more appropriate?  Or we
could make a new more generic error code:

#define HV_STATUS_INVALID1

regards,
dan carpenter

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

2018-04-26 Thread Dan Carpenter

On Wed, Apr 25, 2018 at 11:12:47AM -0700, k...@linuxonhyperv.com wrote:
> +/*
> + * IPI implementation on Hyper-V.
> + */
> +
> +static int __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> + int cur_cpu, vcpu;
> + struct ipi_arg_non_ex **arg;
> + struct ipi_arg_non_ex *ipi_arg;
> + int ret = 1;

Not specifically related to this patch, but hv code sometimes returns 1
on error or U64_MAX.  It's slightly magical.  Maybe
HV_STATUS_INVALID_HYPERCALL_INPUT (3) would be more appropriate?  Or we
could make a new more generic error code:

#define HV_STATUS_INVALID1

regards,
dan carpenter

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

RE: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

Re: [PATCH 2/5] X86: Hyper-V: Enable IPI enlightenments

18 matches

Site Navigation

Mail list logo

Footer information