On Tue, May 30, 2017 at 2:34 PM, Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote:
> Hyper-V host can suggest us to use hypercall for doing remote TLB flush,
> this is supposed to work faster than IPIs.
>
> Implementation details: to do HvFlushVirtualAddress{Space,List} hypercalls
> we need to put the input somewhere in memory and we don't really want to
> have memory allocation on each call so we pre-allocate per cpu memory areas
> on boot. These areas are of fixes size, limit them with an arbitrary number
> of 16 (16 gvas are able to specify 16 * 4096 pages).
>
> pv_ops patching is happening very early so we need to separate
> hyperv_setup_mmu_ops() and hyper_alloc_mmu().
>
> It is possible and easy to implement local TLB flushing too and there is
> even a hint for that. However, I don't see a room for optimization on the
> host side as both hypercall and native tlb flush will result in vmexit. The
> hint is also not set on modern Hyper-V versions.
> @@ -0,0 +1,121 @@
> +#include <linux/types.h>
> +#include <linux/hyperv.h>
> +#include <linux/slab.h>
> +#include <linux/log2.h>
Alphabetical order, please.
+ empty line
> +#include <asm/mshyperv.h>
> +#include <asm/tlbflush.h>
> +#include <asm/msr.h>
> +#include <asm/fpu/api.h>
Can be alphabetically ordered?
> +/* HvFlushVirtualAddressSpace, HvFlushVirtualAddressList hypercalls */
> +struct hv_flush_pcpu {
> + __u64 address_space;
> + __u64 flags;
> + __u64 processor_mask;
> + __u64 gva_list[];
> +};
I dunno what is the style there, but usually in Linux __uXX types are
used exclusively for User API.
Is it a case here? Can we use plain uXX types instead?
> +/* Each gva in gva_list encodes up to 4096 pages to flush */
> +#define HV_TLB_FLUSH_UNIT (PAGE_SIZE * PAGE_SIZE)
Regarding to the comment it would be rather
(4096 * PAGE_SIZE)
Yes, theoretically PAGE_SIZE can be not 4096.
> +static void hyperv_flush_tlb_others(const struct cpumask *cpus,
> + struct mm_struct *mm, unsigned long start,
> + unsigned long end)
> +{
> + if (cpumask_equal(cpus, cpu_present_mask)) {
> + flush->flags |= HV_FLUSH_ALL_PROCESSORS;
> + } else {
> + for_each_cpu(cpu, cpus) {
> + vcpu = hv_cpu_number_to_vp_number(cpu);
> + if (vcpu != -1 && vcpu < 64)
Just
if (vcpu < 64)
?
> + __set_bit(vcpu, (unsigned long *)
> + &flush->processor_mask);
> + else
> + goto do_native;
> + }
> + }
> + if (end == TLB_FLUSH_ALL) {
> + flush->flags |= HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLY;
> + status = hv_do_hypercall(HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE,
> + flush, NULL);
> + } else if (end && ((end - start)/HV_TLB_FLUSH_UNIT) > max_gvas) {
> + status = hv_do_hypercall(HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE,
> + flush, NULL);
Yes! Looks much more cleaner.
> + } else {
> + cur = start;
> + gva_n = 0;
> + 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 + HV_TLB_FLUSH_UNIT)
> + flush->gva_list[gva_n] |= ~PAGE_MASK;
> + else if (end > cur)
> + flush->gva_list[gva_n] |=
> + (end - cur - 1) >> PAGE_SHIFT;
You can also simplify this slightly by introducing
unsigned long diff = end > cur ? end - cur : 0;
if (diff >= HV_TLB_FLUSH_UNIT)
flush->gva_list[gva_n] |= ~PAGE_MASK;
else if (diff)
flush->gva_list[gva_n] |= (diff - 1) >> PAGE_SHIFT;
> +
> + cur += HV_TLB_FLUSH_UNIT;
> + ++gva_n;
Make it post-increment. Better for reader (No need to pay an
additional attention why it's a pre-increment)
> +
> + } while (cur < end);
> + if (!(status & 0xffff))
Not first time I see this magic.
Perhaps
#define STATUS_BLA_BLA_MASK GENMASK(15,0)
if (!(status & STATUS_BLA_BLA_MASK))
in all appropriate places?
> +#define HV_FLUSH_ALL_PROCESSORS 0x00000001
> +#define HV_FLUSH_ALL_VIRTUAL_ADDRESS_SPACES 0x00000002
> +#define HV_FLUSH_NON_GLOBAL_MAPPINGS_ONLY 0x00000004
> +#define HV_FLUSH_USE_EXTENDED_RANGE_FORMAT 0x00000008
BIT() ?
--
With Best Regards,
Andy Shevchenko
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
