Re: [PATCH v4 02/10] x86/hyper-v: stash the max number of virtual/logical processor
On Wed, May 24, 2017 at 3:03 PM, Vitaly Kuznetsov wrote:
> Max virtual processor will be needed for 'extended' hypercalls supporting
> more than 64 vCPUs. While on it, unify on 'Hyper-V' in mshyperv.c as we
> currently have a mix, report acquired misc features as well.
>
> Signed-off-by: Vitaly Kuznetsov
> Acked-by: K. Y. Srinivasan
> Tested-by: Simon Xiao
> Tested-by: Srikanth Myakam
> + u32 max_vp_index;
> + u32 max_lp_index;
> + pr_info("Hyper-V: max %d virtual processors, %d logical processors\n",
> + ms_hyperv.max_vp_index, ms_hyperv.max_lp_index);
And surprisingly no-one from the above list did not get a warning?!
%u, please.
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 03/10] x86/hyper-v: make hv_do_hypercall() inline
On Wed, May 24, 2017 at 3:03 PM, Vitaly Kuznetsov wrote: > We have only three call sites for hv_do_hypercall() and we're going to > change HVCALL_SIGNAL_EVENT to doing fast hypercall so we can inline this > function for optimization. > > Hyper-V top level functional specification states that r9-r11 registers > and flags may be clobbered by the hypervisor during hypercall and with > inlining this is somewhat important, add the clobbers. > + u32 control_hi = control >> 32; > + u32 control_lo = control & 0x; > + u32 input_address_hi = input_address >> 32; > + u32 input_address_lo = input_address & 0x; > + u32 output_address_hi = output_address >> 32; > + u32 output_address_lo = output_address & 0x; Yes, I have noticed it was in older code, but see, all conjunctions above are redundant. Besides that, you may consider to use upper_32_bits() / lower_32_bits() macros. -- With Best Regards, Andy Shevchenko ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 04/10] x86/hyper-v: fast hypercall implementation
On Wed, May 24, 2017 at 3:03 PM, Vitaly Kuznetsov wrote:
> Hyper-V supports 'fast' hypercalls when all parameters are passed through
> registers. Implement an inline version of a simpliest of these calls:
> hypercall with one 8-byte input and no output.
>
> Proper hypercall input interface (struct hv_hypercall_input) definition is
> added as well.
> + u32 hv_status_hi, hv_status_lo;
> + u32 input1_hi = (u32)(input1 >> 32);
> + u32 input1_lo = (u32)input1;
Explicit casting is redundant.
Also consider using macros.
> +/* Hypercall interface */
> +union hv_hypercall_input {
Be careful wrt union aliasing.
> + u64 as_uint64;
> + struct {
> + __u32 as_uint32_lo;
> + __u32 as_uint32_hi;
> + };
> + struct {
> + __u64 code:16;
> + __u64 fast:1;
> + __u64 varhead_size:10;
> + __u64 reserved1:5;
> + __u64 rep_count:12;
> + __u64 reserved2:4;
> + __u64 rep_start:12;
> + __u64 reserved3:4;
> + };
> +};
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 05/10] hyper-v: use fast hypercall for HVCALL_SIGNAL_EVENT
On Wed, May 24, 2017 at 3:04 PM, Vitaly Kuznetsov wrote:
> We need to pass only 8 bytes of input for HvSignalEvent which makes it a
> perfect fit for fast hypercall. hv_input_signal_event_buffer is not needed
> any more and hv_input_signal_event is converted to union for convenience.
> +union hv_input_signal_event {
Union aliasing is UB. Avoid it for good.
> + u64 as_uint64;
> + struct {
> + union hv_connection_id connectionid;
> + u16 flag_number;
> + u16 rsvdz;
> + };
> };
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 09/10] x86/hyper-v: support extended CPU ranges for TLB flush hypercalls
On Wed, May 24, 2017 at 3:04 PM, Vitaly Kuznetsov wrote:
> Hyper-V hosts may support more than 64 vCPUs, we need to use
> HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX/LIST_EX hypercalls in this
> case.
> +{
> + /*
> +* We can't be sure that translated vcpu numbers will always be
> +* in ascending order, so iterate over all possible banks and
> +* check all vcpus in it instead.
vcpu -> vCPU
vcpus -> vCPUs
> +*/
> + for (cur_bank = 0; cur_bank < ms_hyperv.max_vp_index/64; cur_bank++) {
> + has_cpus = false;
> + for_each_cpu(cpu, cpus) {
int vcpu_bank = vcpu / 64;
int vcpu_offset = vcpu % 64;
> + vcpu = hv_cpu_number_to_vp_number(cpu);
> + if (vcpu/64 != cur_bank)
if (vcpu_bank != cur_bank)
> + continue;
> + if (!has_cpus) {
> + flush->hv_vp_set.valid_bank_mask |=
> + 1 << vcpu / 64;
__set_bit(vcpu_bank, &mask);
> + flush->hv_vp_set.bank_contents[nr_bank] =
> + 1 << vcpu % 64;
Ditto. (vcpu_offset)
> + has_cpus = true;
> + } else {
> + flush->hv_vp_set.bank_contents[nr_bank] |=
> + 1 << vcpu % 64;
Ditto.
> + }
> + }
> + if (has_cpus)
> + nr_bank++;
> + }
> +
> + return nr_bank;
> +}
> +static void hyperv_flush_tlb_others_ex(const struct cpumask *cpus,
> + struct mm_struct *mm,
> + unsigned long start,
> + unsigned long end)
> +{
> + struct hv_flush_pcpu_ex *flush;
> + unsigned long cur, flags;
> + u64 status = -1ULL;
U64_MAX
> + int nr_bank = 0, max_gvas, gva_n;
> + /*
> +* We can flush not more than max_gvas with one hypercall. Flush the
> +* whole address space if we were asked to do more.
> +*/
#define XXX(PAGE_SIZE * PAGE_SIZE)
> + max_gvas = (PAGE_SIZE - sizeof(*flush) - nr_bank*8) / 8;
> +
> + if (end == TLB_FLUSH_ALL ||
> + (end && ((end - start)/(PAGE_SIZE*PAGE_SIZE)) > max_gvas)) {
> + if (end == TLB_FLUSH_ALL)
> + flush->flags |= HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLY;
> +
> + status = hv_do_rep_hypercall(
> + HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX,
> + 0, nr_bank + 2, flush, NULL);
if (end == _FLASH_ALL) {
flush->flags |= HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLY;
status = hv_do_rep_hypercall(
HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX,
0, nr_bank + 2, flush, NULL);
} else if (end && ((end - start) / XXX > max_gvas)) {
status = hv_do_rep_hypercall(
HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX,
0, nr_bank + 2, flush, NULL);
} else {
...
Yes, a bit more code, but IMO much more understandable.
> + } else {
> + cur = start;
> + gva_n = nr_bank;
> + do {
> + flush->gva_list[gva_n] = cur & PAGE_MASK;
> + /*
> +* Lower 12 bits encode the number of additional
> +* pages to flush (in addition to the 'cur' page).
> +*/
> + if (end >= cur + PAGE_SIZE * PAGE_SIZE)
if (end >= cur + XXX)
> + flush->gva_list[gva_n] |= ~PAGE_MASK;
> + else if (end > cur)
> + flush->gva_list[gva_n] |=
> + (end - cur - 1) >> PAGE_SHIFT;
> +
> + cur += PAGE_SIZE * PAGE_SIZE;
+= XXX;
> + ++gva_n;
> +
> + } while (cur < end);
> +}
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] drivers: staging: lustre: replace variable length array by dynamic allocation
On Tue, May 23, 2017 at 10:38 PM, Greg Kroah-Hartman wrote: > On Tue, May 23, 2017 at 09:32:13PM +0200, Raphaël Beamonte wrote: >> drivers/staging/lustre/lustre/llite/xattr.c:89:62: warning: Variable >> length array is used. >> drivers/staging/lustre/lustre/llite/xattr.c:366:62: warning: Variable >> length array is used. >> + fullname = kmalloc(strlen(handler->prefix) + strlen(name) + 1, >> +GFP_KERNEL); >> + if (!fullname) >> + return -ENOMEM; >> + >> sprintf(fullname, "%s%s\n", handler->prefix, name); >> rc = md_setxattr(sbi->ll_md_exp, ll_inode2fid(inode), >>valid, fullname, pv, size, 0, flags, >>ll_i2suppgid(inode), &req); >> + >> + kfree(fullname); > > Didn't we reject much this same patch last week? ...besides the fact that kasprintf(); might be better in cases when malloc + snprintf is used. -- With Best Regards, Andy Shevchenko ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
