* Re: [PATCH v2 08/10] KVM: arm64: guest debug, HW assisted debug support
From: Alex Bennée @ 2015-04-13 8:00 UTC (permalink / raw)
To: Andrew Jones
Cc: kvm, open list:DOCUMENTATION, jan.kiszka, Will Deacon, kvmarm,
Lorenzo Pieralisi, Russell King, Jonathan Corbet, AKASHI Takahiro,
zhichao.huang, Catalin Marinas, bp, Gleb Natapov, marc.zyngier,
r65777, linux-arm-kernel, open list:ABI/API, open list, dahi,
pbonzini
In-Reply-To: <20150410122520.GA3227@hawk.usersys.redhat.com>
Andrew Jones <drjones@redhat.com> writes:
> On Tue, Mar 31, 2015 at 04:08:06PM +0100, Alex Bennée wrote:
>> This adds support for userspace to control the HW debug registers for
>> guest debug. We'll only copy the $ARCH defined number across as that is
>> all that hyp.S will use anyway. I've moved some helper functions into
>> the hw_breakpoint.h header for re-use.
>>
>> As with single step we need to tweak the guest registers to enable the
>> exceptions so we need to save and restore those bits.
>>
>> Two new capabilities have been added to the KVM_EXTENSION ioctl to allow
>> userspace to query the number of hardware break and watch points
>> available on the host hardware.
>>
>> As QEMU tests for watchpoints based on the address and not the PC we
>> also need to export the value of far_el2 to userspace.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>
>> ---
>> v2
>> - switched to C setup
>> - replace host debug registers directly into context
>> - minor tweak to api docs
>> - setup right register for debug
>> - add FAR_EL2 to debug exit structure
>> - add support fro trapping debug register access
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 17d4f9c..ac34093 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -2627,7 +2627,7 @@ The top 16 bits of the control field are architecture specific control
>> flags which can include the following:
>>
>> - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
>> - - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
>> + - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64]
>> - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
>> - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
>> - KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390]
>> @@ -2642,6 +2642,10 @@ updated to the correct (supplied) values.
>> The second part of the structure is architecture specific and
>> typically contains a set of debug registers.
>>
>> +For arm64 the number of debug registers is implementation defined and
>> +can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and
>> +KVM_CAP_GUEST_DEBUG_HW_WPS capabilities.
>> +
>> When debug events exit the main run loop with the reason
>> KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run
>> structure containing architecture specific debug information.
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index c1ed8cb..a286026 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -306,6 +306,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>>
>> #define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE | \
>> KVM_GUESTDBG_USE_SW_BP | \
>> + KVM_GUESTDBG_USE_HW_BP | \
>> KVM_GUESTDBG_SINGLESTEP)
>>
>> /**
>> @@ -328,6 +329,26 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
>> return -EINVAL;
>>
>> vcpu->guest_debug = dbg->control;
>> +
>> + /* Hardware assisted Break and Watch points */
>> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
>> + int nb = get_num_brps();
>> + int nw = get_num_wrps();
>> +
>> + /* Copy across up to IMPDEF debug registers to our
>> + * shadow copy in the vcpu structure. The debug code
>> + * will then set them up before we re-enter the guest.
>> + */
>
> Is it necessary to limit the number copied here by anything other than
> what the struct supports? Userspace gave us a struct kvm_guest_debug_arch,
> so let's save the whole thing. How about just
>
> vcpu->arch.guest_debug_regs = *dbg;
Makes sense for the ioctl. I'll still limit it in the setup/clear function.
>
>> + memcpy(vcpu->arch.guest_debug_regs.dbg_bcr,
>> + dbg->arch.dbg_bcr, sizeof(__u64)*nb);
>> + memcpy(vcpu->arch.guest_debug_regs.dbg_bvr,
>> + dbg->arch.dbg_bvr, sizeof(__u64)*nb);
>> + memcpy(vcpu->arch.guest_debug_regs.dbg_wcr,
>> + dbg->arch.dbg_wcr, sizeof(__u64)*nw);
>> + memcpy(vcpu->arch.guest_debug_regs.dbg_wvr,
>> + dbg->arch.dbg_wvr, sizeof(__u64)*nw);
>> + }
>> +
>> } else {
>> /* If not enabled clear all flags */
>> vcpu->guest_debug = 0;
>> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
>> index 52b484b..c450552 100644
>> --- a/arch/arm64/include/asm/hw_breakpoint.h
>> +++ b/arch/arm64/include/asm/hw_breakpoint.h
>> @@ -130,6 +130,18 @@ static inline void ptrace_hw_copy_thread(struct task_struct *task)
>> }
>> #endif
>>
>> +/* Determine number of BRP registers available. */
>> +static inline int get_num_brps(void)
>> +{
>> + return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
>> +}
>> +
>> +/* Determine number of WRP registers available. */
>> +static inline int get_num_wrps(void)
>> +{
>> + return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
>> +}
>> +
>> extern struct pmu perf_ops_bp;
>>
>> #endif /* __KERNEL__ */
>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
>> index 6a33647..2c359c9 100644
>> --- a/arch/arm64/include/asm/kvm_host.h
>> +++ b/arch/arm64/include/asm/kvm_host.h
>> @@ -106,8 +106,9 @@ struct kvm_vcpu_arch {
>> /* Exception Information */
>> struct kvm_vcpu_fault_info fault;
>>
>> - /* Debug state */
>> + /* Guest debug state */
>> u64 debug_flags;
>> + struct kvm_guest_debug_arch guest_debug_regs;
>>
>> /* Pointer to host CPU context */
>> kvm_cpu_context_t *host_cpu_context;
>> @@ -126,6 +127,7 @@ struct kvm_vcpu_arch {
>> u32 pstate_ss_bit;
>> u32 mdscr_el1_bits;
>>
>> + struct kvm_guest_debug_arch debug_regs;
>> } debug_saved_regs;
>>
>> /* Don't run the guest */
>> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
>> index 6ee70a0..73f21e4 100644
>> --- a/arch/arm64/include/uapi/asm/kvm.h
>> +++ b/arch/arm64/include/uapi/asm/kvm.h
>> @@ -118,6 +118,7 @@ struct kvm_guest_debug_arch {
>> struct kvm_debug_exit_arch {
>> __u64 pc;
>> __u32 hsr;
>> + __u64 far; /* used for watchpoints */
>> };
>>
>> struct kvm_sync_regs {
>> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
>> index 98bbe06..923be44 100644
>> --- a/arch/arm64/kernel/hw_breakpoint.c
>> +++ b/arch/arm64/kernel/hw_breakpoint.c
>> @@ -49,18 +49,6 @@ static DEFINE_PER_CPU(int, stepping_kernel_bp);
>> static int core_num_brps;
>> static int core_num_wrps;
>>
>> -/* Determine number of BRP registers available. */
>> -static int get_num_brps(void)
>> -{
>> - return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
>> -}
>> -
>> -/* Determine number of WRP registers available. */
>> -static int get_num_wrps(void)
>> -{
>> - return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
>> -}
>> -
>> int hw_breakpoint_slots(int type)
>> {
>> /*
>> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
>> index b32362c..3b368f3 100644
>> --- a/arch/arm64/kvm/debug.c
>> +++ b/arch/arm64/kvm/debug.c
>> @@ -50,14 +50,19 @@
>>
>> void kvm_arch_setup_debug(struct kvm_vcpu *vcpu)
>> {
>> + bool trap_debug = false;
>> +
>> vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM | MDCR_EL2_TPMCR);
>> vcpu->arch.mdcr_el2 |= (MDCR_EL2_TDRA | MDCR_EL2_TDOSA);
>>
>> - /* Trap debug register access? */
>> + trace_kvm_arch_setup_debug_reg32("MDCR_EL2", vcpu->arch.mdcr_el2);
>
> This is 2 patches too early. You don't introduce this tracepoint until
> later.
>
>> +
>> + /*
>> + * If we are not treating debug registers are dirty we need
> ^^ as
>> + * to trap if the guest starts accessing them.
>> + */
>> if (!vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
>> - vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
>> - else
>> - vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
>> + trap_debug = true;
>
> How about just
>
> bool trap_debug = !(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY);
>
> at the top instead?
>
>>
>> /* Is Guest debugging in effect? */
>> if (vcpu->guest_debug) {
>> @@ -84,10 +89,69 @@ void kvm_arch_setup_debug(struct kvm_vcpu *vcpu)
>> vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
>> }
>>
>> + /*
>> + * HW Break/Watch points
>> + */
>> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
>> + int nb = get_num_brps();
>> + int nw = get_num_wrps();
>> + struct kvm_guest_debug_arch *saved, *host;
>> +
>> + vcpu_sys_reg(vcpu, MDSCR_EL1) |=
>> + (DBG_MDSCR_KDE|DBG_MDSCR_MDE);
>> +
>> + /*
>> + * First we need to make a copy of the guest
>> + * debug registers before we wipe them out
>> + * with the ones we want to use.
>> + */
>> + saved = &vcpu_debug_saved_reg(vcpu, debug_regs);
>> + host = &vcpu->arch.guest_debug_regs;
>
> Again, I don't think we need to be so precise with our copy. We can
> always copy extra without problems. It's just what we use that matters.
> And how about a couple helpers?
>
> debug_regs_save_to(struct kvm_vcpu *vcpu, struct kvm_guest_debug_arch *dst)
> {
> memcpy(dst->dbg_bcr, &vcpu_sys_reg(vcpu, DBGBCR0_EL1), sizeof(u64)*KVM_ARM_NDBG_REGS);
> memcpy(dst->dbg_bvr, &vcpu_sys_reg(vcpu, DBGBVR0_EL1), sizeof(u64)*KVM_ARM_NDBG_REGS);
> memcpy(dst->dbg_wcr, &vcpu_sys_reg(vcpu, DBGWCR0_EL1), sizeof(u64)*KVM_ARM_NDBG_REGS);
> memcpy(dst->dbg_wvr, &vcpu_sys_reg(vcpu, DBGWVR0_EL1), sizeof(u64)*KVM_ARM_NDBG_REGS);
> }
> debug_regs_restore_from(struct kvm_vcpu *vcpu, struct kvm_guest_debug_arch *src)
> {
> memcpy(&vcpu_sys_reg(vcpu, DBGBCR0_EL1), src->dbg_bcr, sizeof(u64)*KVM_ARM_NDBG_REGS);
> memcpy(&vcpu_sys_reg(vcpu, DBGBVR0_EL1), src->dbg_bvr, sizeof(u64)*KVM_ARM_NDBG_REGS);
> memcpy(&vcpu_sys_reg(vcpu, DBGWCR0_EL1), src->dbg_wcr, sizeof(u64)*KVM_ARM_NDBG_REGS);
> memcpy(&vcpu_sys_reg(vcpu, DBGWVR0_EL1), src->dbg_wvr, sizeof(u64)*KVM_ARM_NDBG_REGS);
> }
>
>> +
>> + /* Save guest values */
>> + memcpy(&saved->dbg_bcr,
>> + &vcpu_sys_reg(vcpu, DBGBCR0_EL1),
>> + sizeof(__u64)*nb);
>> + memcpy(&saved->dbg_bvr,
>> + &vcpu_sys_reg(vcpu, DBGBVR0_EL1),
>> + sizeof(__u64)*nb);
>> + memcpy(&saved->dbg_wcr,
>> + &vcpu_sys_reg(vcpu, DBGWCR0_EL1),
>> + sizeof(__u64)*nw);
>> + memcpy(&saved->dbg_wvr,
>> + &vcpu_sys_reg(vcpu, DBGWVR0_EL1),
>> + sizeof(__u64)*nw);
>> +
>> + /* Copy in host values */
>> + memcpy(&vcpu_sys_reg(vcpu, DBGBCR0_EL1),
>> + &host->dbg_bcr,
>> + sizeof(__u64)*nb);
>> + memcpy(&vcpu_sys_reg(vcpu, DBGBVR0_EL1),
>> + &host->dbg_bvr,
>> + sizeof(__u64)*nb);
>> + memcpy(&vcpu_sys_reg(vcpu, DBGWCR0_EL1),
>> + &host->dbg_wcr,
>> + sizeof(__u64)*nw);
>> + memcpy(&vcpu_sys_reg(vcpu, DBGWVR0_EL1),
>> + &host->dbg_wvr,
>> + sizeof(__u64)*nw);
>> +
>> + /* Make sure hyp.S copies them in/out */
>> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
>> + /* Also track guest changes */
>> + trap_debug = true;
>> + }
>> +
>> } else {
>> /* Debug operations can go straight to the guest */
>> vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
>> }
>> +
>> + /* Trap debug register access? */
>> + if (trap_debug)
>> + vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
>> + else
>> + vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
>> }
>>
>> void kvm_arch_clear_debug(struct kvm_vcpu *vcpu)
>> @@ -100,5 +164,31 @@ void kvm_arch_clear_debug(struct kvm_vcpu *vcpu)
>> vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~MDSCR_EL1_DEBUG_BITS;
>> vcpu_sys_reg(vcpu, MDSCR_EL1) |=
>> vcpu_debug_saved_reg(vcpu, mdscr_el1_bits);
>> +
>> + /*
>> + * If we were using HW debug we need to restore the
>> + * values the guest had set them up with
>> + */
>> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
>> + struct kvm_guest_debug_arch *regs;
>> + int nb = get_num_brps();
>> + int nw = get_num_wrps();
>> +
>> + regs = &vcpu_debug_saved_reg(vcpu, debug_regs);
> debug_regs_restore_from(vcpu, regs);
>> +
>> + /* Restore the saved debug register values */
>> + memcpy(&vcpu_sys_reg(vcpu, DBGBCR0_EL1),
>> + ®s->dbg_bcr,
>> + sizeof(__u64)*nb);
>> + memcpy(&vcpu_sys_reg(vcpu, DBGBVR0_EL1),
>> + ®s->dbg_bvr,
>> + sizeof(__u64)*nb);
>> + memcpy(&vcpu_sys_reg(vcpu, DBGWCR0_EL1),
>> + ®s->dbg_wcr,
>> + sizeof(__u64)*nw);
>> + memcpy(&vcpu_sys_reg(vcpu, DBGWVR0_EL1),
>> + ®s->dbg_wvr,
>> + sizeof(__u64)*nw);
>> + }
>> }
>> }
>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
>> index 16accae..460a1aa 100644
>> --- a/arch/arm64/kvm/handle_exit.c
>> +++ b/arch/arm64/kvm/handle_exit.c
>> @@ -101,7 +101,11 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> run->debug.arch.hsr = hsr;
>>
>> switch (hsr >> ESR_ELx_EC_SHIFT) {
>> + case ESR_ELx_EC_WATCHPT_LOW:
>> + run->debug.arch.far = vcpu->arch.fault.far_el2;
>> + /* fall through */
>> case ESR_ELx_EC_SOFTSTP_LOW:
>> + case ESR_ELx_EC_BREAKPT_LOW:
>> case ESR_ELx_EC_BKPT32:
>> case ESR_ELx_EC_BRK64:
>> run->debug.arch.pc = *vcpu_pc(vcpu);
>> @@ -129,6 +133,8 @@ static exit_handle_fn arm_exit_handlers[] = {
>> [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
>> [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
>> [ESR_ELx_EC_SOFTSTP_LOW]= kvm_handle_guest_debug,
>> + [ESR_ELx_EC_WATCHPT_LOW]= kvm_handle_guest_debug,
>> + [ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug,
>> [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
>> [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
>> };
>> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
>> index 0b43265..c2732c7 100644
>> --- a/arch/arm64/kvm/reset.c
>> +++ b/arch/arm64/kvm/reset.c
>> @@ -64,6 +64,12 @@ int kvm_arch_dev_ioctl_check_extension(long ext)
>> case KVM_CAP_ARM_EL1_32BIT:
>> r = cpu_has_32bit_el1();
>> break;
>> + case KVM_CAP_GUEST_DEBUG_HW_BPS:
>> + r = get_num_brps();
>> + break;
>> + case KVM_CAP_GUEST_DEBUG_HW_WPS:
>> + r = get_num_wrps();
>> + break;
>> default:
>> r = 0;
>> }
>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>> index c370b40..be9b188 100644
>> --- a/arch/arm64/kvm/sys_regs.c
>> +++ b/arch/arm64/kvm/sys_regs.c
>> @@ -196,16 +196,49 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
>> * - If the dirty bit is set, save guest registers, restore host
>> * registers and clear the dirty bit. This ensure that the host can
>> * now use the debug registers.
>> + *
>> + * We also use this mechanism to set-up the debug registers for guest
> setup
>> + * debugging. If this is the case we want to ensure the guest sees
>> + * the right versions of the registers - even if they are not going to
>> + * be effective while guest debug is using HW debug.
>> + *
> no need for this blank comment line
>> */
>> +
>> static bool trap_debug_regs(struct kvm_vcpu *vcpu,
>> const struct sys_reg_params *p,
>> const struct sys_reg_desc *r)
>> {
>> - if (p->is_write) {
>> - vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt);
>> - vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
>> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
>> + struct kvm_guest_debug_arch *saved;
>> + __u64 *val;
>> +
>> + saved = &vcpu_debug_saved_reg(vcpu, debug_regs);
>
> Here we don't bother enforcing num_brps/wrps, which is fine by me.
>
>> +
>> + if (r->reg >= DBGBCR0_EL1 && r->reg <= DBGBCR15_EL1)
>> + val = &saved->dbg_bcr[r->reg - DBGBCR0_EL1];
>> + else if (r->reg >= DBGBVR0_EL1 && r->reg <= DBGBVR15_EL1)
>> + val = &saved->dbg_bvr[r->reg - DBGBVR0_EL1];
>> + else if (r->reg >= DBGWCR0_EL1 && r->reg <= DBGWCR15_EL1)
>> + val = &saved->dbg_wcr[r->reg - DBGWCR0_EL1];
>> + else if (r->reg >= DBGWVR0_EL1 && r->reg <= DBGWVR15_EL1)
>> + val = &saved->dbg_wvr[r->reg - DBGWVR0_EL1];
>> + else {
>> + kvm_err("Bad register index %d\n", r->reg);
>> + return false;
>> + }
>> +
>> + if (p->is_write)
>> + *val = *vcpu_reg(vcpu, p->Rt);
>> + else
>> + *vcpu_reg(vcpu, p->Rt) = *val;
>> +
>> } else {
>> - *vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg);
>> + if (p->is_write) {
>> + vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt);
>> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
>> + } else {
>> + *vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg);
>> + }
>> }
>>
>> return true;
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index ce2db14..0e48c0d 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -771,6 +771,8 @@ struct kvm_ppc_smmu_info {
>> #define KVM_CAP_PPC_ENABLE_HCALL 104
>> #define KVM_CAP_CHECK_EXTENSION_VM 105
>> #define KVM_CAP_S390_USER_SIGP 106
>> +#define KVM_CAP_GUEST_DEBUG_HW_BPS 107
>> +#define KVM_CAP_GUEST_DEBUG_HW_WPS 108
>>
>> #ifdef KVM_CAP_IRQ_ROUTING
>>
>> --
>> 2.3.4
>>
--
Alex Bennée
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply
* Re: [v12 0/5] ext4: add project quota support
From: Alban Crequy @ 2015-04-12 15:36 UTC (permalink / raw)
To: Li Xi
Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-ext4-u79uwXL29TY76Z2rM5mHXA, Linux API, tytso-3s7WtUTddSA,
adilger-m1MBpc4rdrD3fQ9qLvQP4Q, jack-AlSwsSmVLrQ,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, hch-wEGCiKHe2LqWVfeAwA7xHQ,
dmonakhov-GEFAQzZX7r8dnm+yROfE0A, Linux Containers
In-Reply-To: <1428592477-8212-1-git-send-email-lixi-LfVdkaOWEx8@public.gmane.org>
On 9 April 2015 at 17:14, Li Xi <pkuelelixi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The following patches propose an implementation of project quota
> support for ext4. A project is an aggregate of unrelated inodes
> which might scatter in different directories. Inodes that belong
> to the same project possess an identical identification i.e.
> 'project ID', just like every inode has its user/group
> identification. The following patches add project quota as
> supplement to the former uer/group quota types.
> (...)
Thanks for this work, I would like to use this for containers. I am
adding containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org in Cc.
To make sure I understand correctly, I will describe the configuration
I have in mind and hopefully someone can tell me if it makes sense.
Containers created by rkt (https://github.com/coreos/rkt) use an
overlay filesystem as root and the lowerdir/upperdir directories are
based on an ext4 filesystem outside of the container's reach. The
lowerdir is the base image, and several container instances can
potentially use the same lowerdir. Each container has its upperdir
containing their changes.
With your patch set, I could assign a different projid to the upperdir
of each container with a specific quota. Then it will limit how much
the container will be able to write. I don't know if the overlay's
workdir would need to have projid too.
When a quota warning is sent on netlink, it is received only in the
initial user namespace and the processes in a different user namespace
will not be able to receive the netlink warnings. The user will only
receive a warning through the control terminal.
Since rkt does not use user namespaces yet, a rkt container could
unfortunately receive quota warnings through netlink concerning the
host or other containers. Or is it restricted to init_net?
quotactl() can be used in a rkt container if the proccesses in the
container can guess somehow which block device is used by the
filesystem hosting the overlay's upperdir and if they can mknod it
somewhere. Usually, containers don't restrict mknod but just restrict
read-write access through the device cgroup. The read-write access is
irrelevant for quotactl(): quotactl() just check that the device node
exists and that it is not on a nodev mount. The nodev check does not
restrict containers here because they usually have a /dev mounted as
tmpfs without the nodev option.
Containers that don't use user namespaces (so no projid mapping) would
be able to query quotas for projid assigned to other containers
(unfortunately). They would be able to change the quota of other
containers if they are privileged enough to be given CAP_SYS_RESOURCE.
Containers using user namespaces would not be able to change any quota
config because they don't have CAP_SYS_RESOURCE in the init user
namespace. If they are configured with a proper projid mapping, they
would only be able to query the projid they are assigned (they could
guess which projid to query by looking at /proc/self/projid_map).
Do you know if someone is working on the documentation? It would be
nice if filesystems/quota.txt could say who can receive the quota
warnings on netlink (which namespace) and if it could give some
information about projid. But maybe this belong to the proc(5) and
user_namespaces(7) manpages as well.
Is there any suggestions how to allocate projid in userspace?
Something like /etc/subprojid similar to /etc/subuid?
Thanks!
Alban
^ permalink raw reply
* Re: [v12 1/5] vfs: adds general codes to enforces project quota limits
From: Alban Crequy @ 2015-04-12 13:49 UTC (permalink / raw)
To: Li Xi
Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-ext4-u79uwXL29TY76Z2rM5mHXA, Linux API, tytso-3s7WtUTddSA,
adilger-m1MBpc4rdrD3fQ9qLvQP4Q, jack-AlSwsSmVLrQ,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, hch-wEGCiKHe2LqWVfeAwA7xHQ,
dmonakhov-GEFAQzZX7r8dnm+yROfE0A
In-Reply-To: <1428592477-8212-2-git-send-email-lixi-LfVdkaOWEx8@public.gmane.org>
On 9 April 2015 at 17:14, Li Xi <pkuelelixi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This patch adds support for a new quota type PRJQUOTA for project quota
> enforcement. Also a new method get_projid() is added into dquot_operations
> structure.
>(...)
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 2aa4151..33b30b1 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -30,7 +30,10 @@ static int check_quotactl_permission(struct super_block *sb, int type, int cmd,
> case Q_XGETQSTATV:
> case Q_XQUOTASYNC:
> break;
> - /* allow to query information for dquots we "own" */
> + /*
> + * allow to query information for dquots we "own"
> + * always allow querying project quota
> + */
I would add a precision in the comment:
always allow querying project quota when the project id is mapped in
the current user namespace.
id is the id in the current user namespace. So quotas for unmapped
users, unmapped groups or unmapped projects cannot be queried.
^ permalink raw reply
* Re: [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix
From: Jonathan Corbet @ 2015-04-11 13:26 UTC (permalink / raw)
To: David Rientjes
Cc: Andrew Morton, Hugh Dickins, Davide Libenzi, Luiz Capitulino,
Shuah Khan, Andrea Arcangeli, Joern Engel, Jianguo Wu,
Eric B Munson, linux-mm, linux-kernel, linux-api, linux-doc
In-Reply-To: <alpine.DEB.2.10.1504091244501.11370@chino.kir.corp.google.com>
On Thu, 9 Apr 2015 12:46:09 -0700 (PDT)
David Rientjes <rientjes@google.com> wrote:
> It's been merged into that tree, but I would still appreciate your
> ack!
Easy enough.
Acked-by: Jonathan Corbet <corbet@lwn.net>
jon
^ permalink raw reply
* Re: [v12 2/5] ext4: adds project ID support
From: Andreas Dilger @ 2015-04-10 23:37 UTC (permalink / raw)
To: Li Xi
Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-ext4-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, tytso-3s7WtUTddSA,
jack-AlSwsSmVLrQ, viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
hch-wEGCiKHe2LqWVfeAwA7xHQ, dmonakhov-GEFAQzZX7r8dnm+yROfE0A
In-Reply-To: <1428592477-8212-3-git-send-email-lixi-LfVdkaOWEx8@public.gmane.org>
On Apr 9, 2015, at 9:14 AM, Li Xi <pkuelelixi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> This patch adds a new internal field of ext4 inode to save project
> identifier. Also a new flag EXT4_INODE_PROJINHERIT is added for
> inheriting project ID from parent directory.
>
> Signed-off-by: Li Xi <lixi-LfVdkaOWEx8@public.gmane.org>
> Reviewed-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
> ---
> fs/ext4/ext4.h | 21 +++++++++++++++++----
> fs/ext4/ialloc.c | 5 +++++
> fs/ext4/inode.c | 29 +++++++++++++++++++++++++++++
> fs/ext4/namei.c | 18 ++++++++++++++++++
> fs/ext4/super.c | 1 +
> include/uapi/linux/fs.h | 1 +
> 6 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 7fec2ef..7acb2da 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -378,16 +378,18 @@ struct flex_groups {
> #define EXT4_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> #define EXT4_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
> #define EXT4_INLINE_DATA_FL 0x10000000 /* Inode has inline data. */
> +#define EXT4_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> #define EXT4_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
>
> -#define EXT4_FL_USER_VISIBLE 0x004BDFFF /* User visible flags */
> -#define EXT4_FL_USER_MODIFIABLE 0x004380FF /* User modifiable flags */
> +#define EXT4_FL_USER_VISIBLE 0x204BDFFF /* User visible flags */
> +#define EXT4_FL_USER_MODIFIABLE 0x204380FF /* User modifiable flags */
I just noticed that EXT4_INLINE_DATA_FL isn't in EXT4_FL_USER_VISIBLE,
but it probably should be. If this patch is refreshed it wouldn't hurt
to add that flag also.
> /* Flags that should be inherited by new inodes from their parent. */
> #define EXT4_FL_INHERITED (EXT4_SECRM_FL | EXT4_UNRM_FL | EXT4_COMPR_FL |\
> EXT4_SYNC_FL | EXT4_NODUMP_FL | EXT4_NOATIME_FL |\
> EXT4_NOCOMPR_FL | EXT4_JOURNAL_DATA_FL |\
> - EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL)
> + EXT4_NOTAIL_FL | EXT4_DIRSYNC_FL |\
> + EXT4_PROJINHERIT_FL)
>
> /* Flags that are appropriate for regular files (all but dir-specific ones). */
> #define EXT4_REG_FLMASK (~(EXT4_DIRSYNC_FL | EXT4_TOPDIR_FL))
> @@ -435,6 +437,7 @@ enum {
> EXT4_INODE_EA_INODE = 21, /* Inode used for large EA */
> EXT4_INODE_EOFBLOCKS = 22, /* Blocks allocated beyond EOF */
> EXT4_INODE_INLINE_DATA = 28, /* Data in inode. */
> + EXT4_INODE_PROJINHERIT = 29, /* Create with parents projid */
> EXT4_INODE_RESERVED = 31, /* reserved for ext4 lib */
> };
>
> @@ -684,6 +687,7 @@ struct ext4_inode {
> __le32 i_crtime; /* File Creation time */
> __le32 i_crtime_extra; /* extra FileCreationtime (nsec << 2 | epoch) */
> __le32 i_version_hi; /* high 32 bits for 64-bit version */
> + __le32 i_projid; /* Project ID */
> };
>
> struct move_extent {
> @@ -939,6 +943,7 @@ struct ext4_inode_info {
>
> /* Precomputed uuid+inum+igen checksum for seeding inode checksums */
> __u32 i_csum_seed;
> + kprojid_t i_projid;
> };
>
> /*
> @@ -1531,6 +1536,7 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
> */
> #define EXT4_FEATURE_RO_COMPAT_METADATA_CSUM 0x0400
> #define EXT4_FEATURE_RO_COMPAT_READONLY 0x1000
> +#define EXT4_FEATURE_RO_COMPAT_PROJECT 0x2000
>
> #define EXT4_FEATURE_INCOMPAT_COMPRESSION 0x0001
> #define EXT4_FEATURE_INCOMPAT_FILETYPE 0x0002
> @@ -1581,7 +1587,8 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
> EXT4_FEATURE_RO_COMPAT_HUGE_FILE |\
> EXT4_FEATURE_RO_COMPAT_BIGALLOC |\
> EXT4_FEATURE_RO_COMPAT_METADATA_CSUM|\
> - EXT4_FEATURE_RO_COMPAT_QUOTA)
> + EXT4_FEATURE_RO_COMPAT_QUOTA |\
> + EXT4_FEATURE_RO_COMPAT_PROJECT)
>
> /*
> * Default values for user and/or group using reserved blocks
> @@ -1589,6 +1596,11 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
> #define EXT4_DEF_RESUID 0
> #define EXT4_DEF_RESGID 0
>
> +/*
> + * Default project ID
> + */
> +#define EXT4_DEF_PROJID 0
> +
> #define EXT4_DEF_INODE_READAHEAD_BLKS 32
>
> /*
> @@ -2141,6 +2153,7 @@ extern int ext4_zero_partial_blocks(handle_t *handle, struct inode *inode,
> loff_t lstart, loff_t lend);
> extern int ext4_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf);
> extern qsize_t *ext4_get_reserved_space(struct inode *inode);
> +extern int ext4_get_projid(struct inode *inode, kprojid_t *projid);
> extern void ext4_da_update_reserve_space(struct inode *inode,
> int used, int quota_claim);
>
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index ac644c3..10ca9dd 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -756,6 +756,11 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
> inode->i_gid = dir->i_gid;
> } else
> inode_init_owner(inode, dir, mode);
> + if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> + ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT))
> + ei->i_projid = EXT4_I(dir)->i_projid;
> + else
> + ei->i_projid = make_kprojid(&init_user_ns, EXT4_DEF_PROJID);
> dquot_initialize(inode);
>
> if (!goal)
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 4df6d01..6e4833f 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3870,6 +3870,14 @@ static inline void ext4_iget_extra_inode(struct inode *inode,
> EXT4_I(inode)->i_inline_off = 0;
> }
>
> +int ext4_get_projid(struct inode *inode, kprojid_t *projid)
> +{
> + if (!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb, EXT4_FEATURE_RO_COMPAT_PROJECT))
> + return -EOPNOTSUPP;
> + *projid = EXT4_I(inode)->i_projid;
> + return 0;
> +}
> +
> struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
> {
> struct ext4_iloc iloc;
> @@ -3881,6 +3889,7 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
> int block;
> uid_t i_uid;
> gid_t i_gid;
> + projid_t i_projid;
>
> inode = iget_locked(sb, ino);
> if (!inode)
> @@ -3930,12 +3939,18 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
> inode->i_mode = le16_to_cpu(raw_inode->i_mode);
> i_uid = (uid_t)le16_to_cpu(raw_inode->i_uid_low);
> i_gid = (gid_t)le16_to_cpu(raw_inode->i_gid_low);
> + if (EXT4_HAS_RO_COMPAT_FEATURE(sb, EXT4_FEATURE_RO_COMPAT_PROJECT))
> + i_projid = (projid_t)le32_to_cpu(raw_inode->i_projid);
> + else
> + i_projid = EXT4_DEF_PROJID;
> +
> if (!(test_opt(inode->i_sb, NO_UID32))) {
> i_uid |= le16_to_cpu(raw_inode->i_uid_high) << 16;
> i_gid |= le16_to_cpu(raw_inode->i_gid_high) << 16;
> }
> i_uid_write(inode, i_uid);
> i_gid_write(inode, i_gid);
> + ei->i_projid = make_kprojid(&init_user_ns, i_projid);;
> set_nlink(inode, le16_to_cpu(raw_inode->i_links_count));
>
> ext4_clear_state_flags(ei); /* Only relevant on 32-bit archs */
> @@ -4165,6 +4180,7 @@ static int ext4_do_update_inode(handle_t *handle,
> int need_datasync = 0, set_large_file = 0;
> uid_t i_uid;
> gid_t i_gid;
> + projid_t i_projid;
>
> spin_lock(&ei->i_raw_lock);
>
> @@ -4177,6 +4193,7 @@ static int ext4_do_update_inode(handle_t *handle,
> raw_inode->i_mode = cpu_to_le16(inode->i_mode);
> i_uid = i_uid_read(inode);
> i_gid = i_gid_read(inode);
> + i_projid = from_kprojid(&init_user_ns, ei->i_projid);
> if (!(test_opt(inode->i_sb, NO_UID32))) {
> raw_inode->i_uid_low = cpu_to_le16(low_16_bits(i_uid));
> raw_inode->i_gid_low = cpu_to_le16(low_16_bits(i_gid));
> @@ -4256,6 +4273,18 @@ static int ext4_do_update_inode(handle_t *handle,
> }
> }
>
> + BUG_ON(!EXT4_HAS_RO_COMPAT_FEATURE(inode->i_sb,
> + EXT4_FEATURE_RO_COMPAT_PROJECT) &&
> + i_projid != EXT4_DEF_PROJID);
> + if (i_projid != EXT4_DEF_PROJID &&
> + (EXT4_INODE_SIZE(inode->i_sb) <= EXT4_GOOD_OLD_INODE_SIZE ||
> + (!EXT4_FITS_IN_INODE(raw_inode, ei, i_projid)))) {
> + spin_unlock(&ei->i_raw_lock);
> + err = -EFBIG;
I mentioned the following back on v8 of the patch, but I'll write it again:
I don't think -EFBIG "File too large" is a good error here. Better would
be -EOPNOTSUPP for EXT4_INODE_SIZE() <= EXT4_GOOD_OLD_INODE_SIZE, since
this will never work for this filesystem. For the !EXT4_FITS_IN_INODE()
case -EOVERFLOW would be better?
Also, returning the error from ext4_mark_iloc_dirty->ext4_do_update_inode()
is a bit late in the game, since the inode has already been modified at
this point. That callpath typically only returned an error if the disk
was bad or the journal aborted (again normally because the disk was bad),
so at that point the dirty in-memory data was lost anyway.
It would be better to check this in the caller before the inode is changed
so that an error can be returned without (essentially) corrupting the
in-memory state. Since the projid should only be set for new inodes (which
will always have enough space, assuming RO_COMPAT_PROJECT cannot be set on
filesystems with 128-byte inodes), or in case of rename into a project
directory, it shouldn't be too big a change.
> + goto out_brelse;
> + }
> + raw_inode->i_projid = cpu_to_le32(i_projid);
> +
> ext4_inode_csum_set(inode, raw_inode, ei);
>
> spin_unlock(&ei->i_raw_lock);
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 2291923..63a9623 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2938,6 +2938,11 @@ static int ext4_link(struct dentry *old_dentry,
> if (inode->i_nlink >= EXT4_LINK_MAX)
> return -EMLINK;
>
> + if ((ext4_test_inode_flag(dir, EXT4_INODE_PROJINHERIT)) &&
> + (!projid_eq(EXT4_I(dir)->i_projid,
> + EXT4_I(old_dentry->d_inode)->i_projid)))
> + return -EXDEV;
> +
> dquot_initialize(dir);
>
> retry:
> @@ -3217,6 +3222,11 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> int credits;
> u8 old_file_type;
>
> + if ((ext4_test_inode_flag(new_dir, EXT4_INODE_PROJINHERIT)) &&
> + (!projid_eq(EXT4_I(new_dir)->i_projid,
> + EXT4_I(old_dentry->d_inode)->i_projid)))
> + return -EXDEV;
> +
> dquot_initialize(old.dir);
> dquot_initialize(new.dir);
>
> @@ -3395,6 +3405,14 @@ static int ext4_cross_rename(struct inode *old_dir, struct dentry *old_dentry,
> u8 new_file_type;
> int retval;
>
> + if ((ext4_test_inode_flag(new_dir, EXT4_INODE_PROJINHERIT) &&
> + !projid_eq(EXT4_I(new_dir)->i_projid,
> + EXT4_I(old_dentry->d_inode)->i_projid)) ||
> + (ext4_test_inode_flag(old_dir, EXT4_INODE_PROJINHERIT) &&
> + !projid_eq(EXT4_I(old_dir)->i_projid,
> + EXT4_I(new_dentry->d_inode)->i_projid)))
> + return -EXDEV;
> +
> dquot_initialize(old.dir);
> dquot_initialize(new.dir);
>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index bff3427..04c6cc3 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1073,6 +1073,7 @@ static const struct dquot_operations ext4_quota_operations = {
> .write_info = ext4_write_info,
> .alloc_dquot = dquot_alloc,
> .destroy_dquot = dquot_destroy,
> + .get_projid = ext4_get_projid,
> };
>
> static const struct quotactl_ops ext4_qctl_operations = {
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 3735fa0..fcbf647 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -195,6 +195,7 @@ struct inodes_stat_t {
> #define FS_EXTENT_FL 0x00080000 /* Extents */
> #define FS_DIRECTIO_FL 0x00100000 /* Use direct i/o */
> #define FS_NOCOW_FL 0x00800000 /* Do not cow file */
> +#define FS_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> #define FS_RESERVED_FL 0x80000000 /* reserved for ext2 lib */
>
> #define FS_FL_USER_VISIBLE 0x0003DFFF /* User visible flags */
> --
> 1.7.1
>
Cheers, Andreas
^ permalink raw reply
* Re: [PATCH v3 03/21] Drivers: hv: kvp: move poll_channel() to hyperv_vmbus.h
From: Dan Carpenter @ 2015-04-10 18:27 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Radim Krcmar, Greg Kroah-Hartman, Haiyang Zhang, linux-kernel,
linux-api, devel
In-Reply-To: <1428568730-6030-4-git-send-email-vkuznets@redhat.com>
On Thu, Apr 09, 2015 at 10:38:32AM +0200, Vitaly Kuznetsov wrote:
> ..., make it inline and rename it to hv_poll_channel() so it can be reused
> in other hv_util modules.
>
The "...," is fine in the git log but I noticed recently that if you're
reading patches in a web archive then often the subject and the body are
several lines apart.
http://marc.info/?l=linux-api&m=142856917031730&w=2
It's awkward to follow.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v2 02/10] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
From: Andrew Jones @ 2015-04-10 12:59 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, Alexey Kardashevskiy, Benjamin Herrenschmidt, Paul Mackerras,
H. Peter Anvin, kvmarm, Nadav Amit, Michael Ellerman,
maintainer:X86 ARCHITECTURE..., Gleb Natapov, Ingo Molnar,
zhichao.huang, jan.kiszka, bp, marc.zyngier, r65777,
Thomas Gleixner, linux-arm-kernel, open list:ABI/API, open list,
dahi, pbonzini, open list:LINUX FOR POWERPC...
In-Reply-To: <1427814488-28467-3-git-send-email-alex.bennee@linaro.org>
On Tue, Mar 31, 2015 at 04:08:00PM +0100, Alex Bennée wrote:
> Currently x86, powerpc and soon arm64 use the same two architecture
> specific bits for guest debug support for software and hardware
> breakpoints. This makes the shared values explicit while leaving the
> gate open for another architecture to use some other value if they
> really really want to.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index ab4d473..1731569 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -310,8 +310,8 @@ struct kvm_guest_debug_arch {
> * and upper 16 bits are architecture specific. Architecture specific defines
> * that ioctl is for setting hardware breakpoint or software breakpoint.
> */
> -#define KVM_GUESTDBG_USE_SW_BP 0x00010000
> -#define KVM_GUESTDBG_USE_HW_BP 0x00020000
> +#define KVM_GUESTDBG_USE_SW_BP __KVM_GUESTDBG_USE_SW_BP
> +#define KVM_GUESTDBG_USE_HW_BP __KVM_GUESTDBG_USE_HW_BP
>
> /* definition of registers in kvm_run */
> struct kvm_sync_regs {
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index d7dcef5..1438202 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -250,8 +250,8 @@ struct kvm_debug_exit_arch {
> __u64 dr7;
> };
>
> -#define KVM_GUESTDBG_USE_SW_BP 0x00010000
> -#define KVM_GUESTDBG_USE_HW_BP 0x00020000
> +#define KVM_GUESTDBG_USE_SW_BP __KVM_GUESTDBG_USE_SW_BP
> +#define KVM_GUESTDBG_USE_HW_BP __KVM_GUESTDBG_USE_HW_BP
> #define KVM_GUESTDBG_INJECT_DB 0x00040000
> #define KVM_GUESTDBG_INJECT_BP 0x00080000
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 5eedf84..ce2db14 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -525,8 +525,16 @@ struct kvm_s390_irq {
>
> /* for KVM_SET_GUEST_DEBUG */
>
> -#define KVM_GUESTDBG_ENABLE 0x00000001
> -#define KVM_GUESTDBG_SINGLESTEP 0x00000002
> +#define KVM_GUESTDBG_ENABLE (1 << 0)
> +#define KVM_GUESTDBG_SINGLESTEP (1 << 1)
> +
> +/*
> + * Architecture specific stuff uses the top 16 bits of the field,
> + * however there is some shared commonality for the common cases
> + */
> +#define __KVM_GUESTDBG_USE_SW_BP (1 << 16)
> +#define __KVM_GUESTDBG_USE_HW_BP (1 << 17)
> +
>
> struct kvm_guest_debug {
> __u32 control;
> --
> 2.3.4
>
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 01/10] KVM: add commentary for kvm_debug_exit_arch struct
From: Andrew Jones @ 2015-04-10 12:58 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, pbonzini, zhichao.huang, jan.kiszka, dahi,
r65777, bp, Gleb Natapov, open list:ABI/API, open list
In-Reply-To: <1427814488-28467-2-git-send-email-alex.bennee@linaro.org>
On Tue, Mar 31, 2015 at 04:07:59PM +0100, Alex Bennée wrote:
> Bring into line with the commentary for the other structures and their
> KVM_EXIT_* cases.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
>
> v2
> - add comments for other exit types
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 8055706..5eedf84 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -226,6 +226,7 @@ struct kvm_run {
> __u32 count;
> __u64 data_offset; /* relative to kvm_run start */
> } io;
> + /* KVM_EXIT_DEBUG */
> struct {
> struct kvm_debug_exit_arch arch;
> } debug;
> @@ -274,6 +275,7 @@ struct kvm_run {
> __u32 data;
> __u8 is_write;
> } dcr;
> + /* KVM_EXIT_INTERNAL_ERROR */
> struct {
> __u32 suberror;
> /* Available with KVM_CAP_INTERNAL_ERROR_DATA: */
> @@ -284,6 +286,7 @@ struct kvm_run {
> struct {
> __u64 gprs[32];
> } osi;
> + /* KVM_EXIT_PAPR_HCALL */
> struct {
> __u64 nr;
> __u64 ret;
> --
> 2.3.4
>
I echo David's commit message tweak, otherwise
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 08/10] KVM: arm64: guest debug, HW assisted debug support
From: Andrew Jones @ 2015-04-10 12:25 UTC (permalink / raw)
To: Alex Bennée
Cc: kvm, open list:DOCUMENTATION, jan.kiszka, Will Deacon, kvmarm,
Lorenzo Pieralisi, Russell King, Jonathan Corbet, AKASHI Takahiro,
zhichao.huang, Catalin Marinas, bp, Gleb Natapov, marc.zyngier,
r65777, linux-arm-kernel, open list:ABI/API, open list, dahi,
pbonzini
In-Reply-To: <1427814488-28467-9-git-send-email-alex.bennee@linaro.org>
On Tue, Mar 31, 2015 at 04:08:06PM +0100, Alex Bennée wrote:
> This adds support for userspace to control the HW debug registers for
> guest debug. We'll only copy the $ARCH defined number across as that is
> all that hyp.S will use anyway. I've moved some helper functions into
> the hw_breakpoint.h header for re-use.
>
> As with single step we need to tweak the guest registers to enable the
> exceptions so we need to save and restore those bits.
>
> Two new capabilities have been added to the KVM_EXTENSION ioctl to allow
> userspace to query the number of hardware break and watch points
> available on the host hardware.
>
> As QEMU tests for watchpoints based on the address and not the PC we
> also need to export the value of far_el2 to userspace.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2
> - switched to C setup
> - replace host debug registers directly into context
> - minor tweak to api docs
> - setup right register for debug
> - add FAR_EL2 to debug exit structure
> - add support fro trapping debug register access
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 17d4f9c..ac34093 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2627,7 +2627,7 @@ The top 16 bits of the control field are architecture specific control
> flags which can include the following:
>
> - KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
> - - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
> + - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64]
> - KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
> - KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
> - KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390]
> @@ -2642,6 +2642,10 @@ updated to the correct (supplied) values.
> The second part of the structure is architecture specific and
> typically contains a set of debug registers.
>
> +For arm64 the number of debug registers is implementation defined and
> +can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and
> +KVM_CAP_GUEST_DEBUG_HW_WPS capabilities.
> +
> When debug events exit the main run loop with the reason
> KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run
> structure containing architecture specific debug information.
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index c1ed8cb..a286026 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -306,6 +306,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>
> #define KVM_GUESTDBG_VALID (KVM_GUESTDBG_ENABLE | \
> KVM_GUESTDBG_USE_SW_BP | \
> + KVM_GUESTDBG_USE_HW_BP | \
> KVM_GUESTDBG_SINGLESTEP)
>
> /**
> @@ -328,6 +329,26 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> return -EINVAL;
>
> vcpu->guest_debug = dbg->control;
> +
> + /* Hardware assisted Break and Watch points */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> + int nb = get_num_brps();
> + int nw = get_num_wrps();
> +
> + /* Copy across up to IMPDEF debug registers to our
> + * shadow copy in the vcpu structure. The debug code
> + * will then set them up before we re-enter the guest.
> + */
Is it necessary to limit the number copied here by anything other than
what the struct supports? Userspace gave us a struct kvm_guest_debug_arch,
so let's save the whole thing. How about just
vcpu->arch.guest_debug_regs = *dbg;
> + memcpy(vcpu->arch.guest_debug_regs.dbg_bcr,
> + dbg->arch.dbg_bcr, sizeof(__u64)*nb);
> + memcpy(vcpu->arch.guest_debug_regs.dbg_bvr,
> + dbg->arch.dbg_bvr, sizeof(__u64)*nb);
> + memcpy(vcpu->arch.guest_debug_regs.dbg_wcr,
> + dbg->arch.dbg_wcr, sizeof(__u64)*nw);
> + memcpy(vcpu->arch.guest_debug_regs.dbg_wvr,
> + dbg->arch.dbg_wvr, sizeof(__u64)*nw);
> + }
> +
> } else {
> /* If not enabled clear all flags */
> vcpu->guest_debug = 0;
> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
> index 52b484b..c450552 100644
> --- a/arch/arm64/include/asm/hw_breakpoint.h
> +++ b/arch/arm64/include/asm/hw_breakpoint.h
> @@ -130,6 +130,18 @@ static inline void ptrace_hw_copy_thread(struct task_struct *task)
> }
> #endif
>
> +/* Determine number of BRP registers available. */
> +static inline int get_num_brps(void)
> +{
> + return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
> +}
> +
> +/* Determine number of WRP registers available. */
> +static inline int get_num_wrps(void)
> +{
> + return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
> +}
> +
> extern struct pmu perf_ops_bp;
>
> #endif /* __KERNEL__ */
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 6a33647..2c359c9 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -106,8 +106,9 @@ struct kvm_vcpu_arch {
> /* Exception Information */
> struct kvm_vcpu_fault_info fault;
>
> - /* Debug state */
> + /* Guest debug state */
> u64 debug_flags;
> + struct kvm_guest_debug_arch guest_debug_regs;
>
> /* Pointer to host CPU context */
> kvm_cpu_context_t *host_cpu_context;
> @@ -126,6 +127,7 @@ struct kvm_vcpu_arch {
> u32 pstate_ss_bit;
> u32 mdscr_el1_bits;
>
> + struct kvm_guest_debug_arch debug_regs;
> } debug_saved_regs;
>
> /* Don't run the guest */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 6ee70a0..73f21e4 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -118,6 +118,7 @@ struct kvm_guest_debug_arch {
> struct kvm_debug_exit_arch {
> __u64 pc;
> __u32 hsr;
> + __u64 far; /* used for watchpoints */
> };
>
> struct kvm_sync_regs {
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index 98bbe06..923be44 100644
> --- a/arch/arm64/kernel/hw_breakpoint.c
> +++ b/arch/arm64/kernel/hw_breakpoint.c
> @@ -49,18 +49,6 @@ static DEFINE_PER_CPU(int, stepping_kernel_bp);
> static int core_num_brps;
> static int core_num_wrps;
>
> -/* Determine number of BRP registers available. */
> -static int get_num_brps(void)
> -{
> - return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
> -}
> -
> -/* Determine number of WRP registers available. */
> -static int get_num_wrps(void)
> -{
> - return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
> -}
> -
> int hw_breakpoint_slots(int type)
> {
> /*
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index b32362c..3b368f3 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -50,14 +50,19 @@
>
> void kvm_arch_setup_debug(struct kvm_vcpu *vcpu)
> {
> + bool trap_debug = false;
> +
> vcpu->arch.mdcr_el2 |= (MDCR_EL2_TPM | MDCR_EL2_TPMCR);
> vcpu->arch.mdcr_el2 |= (MDCR_EL2_TDRA | MDCR_EL2_TDOSA);
>
> - /* Trap debug register access? */
> + trace_kvm_arch_setup_debug_reg32("MDCR_EL2", vcpu->arch.mdcr_el2);
This is 2 patches too early. You don't introduce this tracepoint until
later.
> +
> + /*
> + * If we are not treating debug registers are dirty we need
^^ as
> + * to trap if the guest starts accessing them.
> + */
> if (!vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY)
> - vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
> - else
> - vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
> + trap_debug = true;
How about just
bool trap_debug = !(vcpu->arch.debug_flags & KVM_ARM64_DEBUG_DIRTY);
at the top instead?
>
> /* Is Guest debugging in effect? */
> if (vcpu->guest_debug) {
> @@ -84,10 +89,69 @@ void kvm_arch_setup_debug(struct kvm_vcpu *vcpu)
> vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
> }
>
> + /*
> + * HW Break/Watch points
> + */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> + int nb = get_num_brps();
> + int nw = get_num_wrps();
> + struct kvm_guest_debug_arch *saved, *host;
> +
> + vcpu_sys_reg(vcpu, MDSCR_EL1) |=
> + (DBG_MDSCR_KDE|DBG_MDSCR_MDE);
> +
> + /*
> + * First we need to make a copy of the guest
> + * debug registers before we wipe them out
> + * with the ones we want to use.
> + */
> + saved = &vcpu_debug_saved_reg(vcpu, debug_regs);
> + host = &vcpu->arch.guest_debug_regs;
Again, I don't think we need to be so precise with our copy. We can
always copy extra without problems. It's just what we use that matters.
And how about a couple helpers?
debug_regs_save_to(struct kvm_vcpu *vcpu, struct kvm_guest_debug_arch *dst)
{
memcpy(dst->dbg_bcr, &vcpu_sys_reg(vcpu, DBGBCR0_EL1), sizeof(u64)*KVM_ARM_NDBG_REGS);
memcpy(dst->dbg_bvr, &vcpu_sys_reg(vcpu, DBGBVR0_EL1), sizeof(u64)*KVM_ARM_NDBG_REGS);
memcpy(dst->dbg_wcr, &vcpu_sys_reg(vcpu, DBGWCR0_EL1), sizeof(u64)*KVM_ARM_NDBG_REGS);
memcpy(dst->dbg_wvr, &vcpu_sys_reg(vcpu, DBGWVR0_EL1), sizeof(u64)*KVM_ARM_NDBG_REGS);
}
debug_regs_restore_from(struct kvm_vcpu *vcpu, struct kvm_guest_debug_arch *src)
{
memcpy(&vcpu_sys_reg(vcpu, DBGBCR0_EL1), src->dbg_bcr, sizeof(u64)*KVM_ARM_NDBG_REGS);
memcpy(&vcpu_sys_reg(vcpu, DBGBVR0_EL1), src->dbg_bvr, sizeof(u64)*KVM_ARM_NDBG_REGS);
memcpy(&vcpu_sys_reg(vcpu, DBGWCR0_EL1), src->dbg_wcr, sizeof(u64)*KVM_ARM_NDBG_REGS);
memcpy(&vcpu_sys_reg(vcpu, DBGWVR0_EL1), src->dbg_wvr, sizeof(u64)*KVM_ARM_NDBG_REGS);
}
> +
> + /* Save guest values */
> + memcpy(&saved->dbg_bcr,
> + &vcpu_sys_reg(vcpu, DBGBCR0_EL1),
> + sizeof(__u64)*nb);
> + memcpy(&saved->dbg_bvr,
> + &vcpu_sys_reg(vcpu, DBGBVR0_EL1),
> + sizeof(__u64)*nb);
> + memcpy(&saved->dbg_wcr,
> + &vcpu_sys_reg(vcpu, DBGWCR0_EL1),
> + sizeof(__u64)*nw);
> + memcpy(&saved->dbg_wvr,
> + &vcpu_sys_reg(vcpu, DBGWVR0_EL1),
> + sizeof(__u64)*nw);
> +
> + /* Copy in host values */
> + memcpy(&vcpu_sys_reg(vcpu, DBGBCR0_EL1),
> + &host->dbg_bcr,
> + sizeof(__u64)*nb);
> + memcpy(&vcpu_sys_reg(vcpu, DBGBVR0_EL1),
> + &host->dbg_bvr,
> + sizeof(__u64)*nb);
> + memcpy(&vcpu_sys_reg(vcpu, DBGWCR0_EL1),
> + &host->dbg_wcr,
> + sizeof(__u64)*nw);
> + memcpy(&vcpu_sys_reg(vcpu, DBGWVR0_EL1),
> + &host->dbg_wvr,
> + sizeof(__u64)*nw);
> +
> + /* Make sure hyp.S copies them in/out */
> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> + /* Also track guest changes */
> + trap_debug = true;
> + }
> +
> } else {
> /* Debug operations can go straight to the guest */
> vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDE;
> }
> +
> + /* Trap debug register access? */
> + if (trap_debug)
> + vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
> + else
> + vcpu->arch.mdcr_el2 &= ~MDCR_EL2_TDA;
> }
>
> void kvm_arch_clear_debug(struct kvm_vcpu *vcpu)
> @@ -100,5 +164,31 @@ void kvm_arch_clear_debug(struct kvm_vcpu *vcpu)
> vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~MDSCR_EL1_DEBUG_BITS;
> vcpu_sys_reg(vcpu, MDSCR_EL1) |=
> vcpu_debug_saved_reg(vcpu, mdscr_el1_bits);
> +
> + /*
> + * If we were using HW debug we need to restore the
> + * values the guest had set them up with
> + */
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> + struct kvm_guest_debug_arch *regs;
> + int nb = get_num_brps();
> + int nw = get_num_wrps();
> +
> + regs = &vcpu_debug_saved_reg(vcpu, debug_regs);
debug_regs_restore_from(vcpu, regs);
> +
> + /* Restore the saved debug register values */
> + memcpy(&vcpu_sys_reg(vcpu, DBGBCR0_EL1),
> + ®s->dbg_bcr,
> + sizeof(__u64)*nb);
> + memcpy(&vcpu_sys_reg(vcpu, DBGBVR0_EL1),
> + ®s->dbg_bvr,
> + sizeof(__u64)*nb);
> + memcpy(&vcpu_sys_reg(vcpu, DBGWCR0_EL1),
> + ®s->dbg_wcr,
> + sizeof(__u64)*nw);
> + memcpy(&vcpu_sys_reg(vcpu, DBGWVR0_EL1),
> + ®s->dbg_wvr,
> + sizeof(__u64)*nw);
> + }
> }
> }
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 16accae..460a1aa 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -101,7 +101,11 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
> run->debug.arch.hsr = hsr;
>
> switch (hsr >> ESR_ELx_EC_SHIFT) {
> + case ESR_ELx_EC_WATCHPT_LOW:
> + run->debug.arch.far = vcpu->arch.fault.far_el2;
> + /* fall through */
> case ESR_ELx_EC_SOFTSTP_LOW:
> + case ESR_ELx_EC_BREAKPT_LOW:
> case ESR_ELx_EC_BKPT32:
> case ESR_ELx_EC_BRK64:
> run->debug.arch.pc = *vcpu_pc(vcpu);
> @@ -129,6 +133,8 @@ static exit_handle_fn arm_exit_handlers[] = {
> [ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
> [ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
> [ESR_ELx_EC_SOFTSTP_LOW]= kvm_handle_guest_debug,
> + [ESR_ELx_EC_WATCHPT_LOW]= kvm_handle_guest_debug,
> + [ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug,
> [ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
> [ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
> };
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 0b43265..c2732c7 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -64,6 +64,12 @@ int kvm_arch_dev_ioctl_check_extension(long ext)
> case KVM_CAP_ARM_EL1_32BIT:
> r = cpu_has_32bit_el1();
> break;
> + case KVM_CAP_GUEST_DEBUG_HW_BPS:
> + r = get_num_brps();
> + break;
> + case KVM_CAP_GUEST_DEBUG_HW_WPS:
> + r = get_num_wrps();
> + break;
> default:
> r = 0;
> }
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index c370b40..be9b188 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -196,16 +196,49 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
> * - If the dirty bit is set, save guest registers, restore host
> * registers and clear the dirty bit. This ensure that the host can
> * now use the debug registers.
> + *
> + * We also use this mechanism to set-up the debug registers for guest
setup
> + * debugging. If this is the case we want to ensure the guest sees
> + * the right versions of the registers - even if they are not going to
> + * be effective while guest debug is using HW debug.
> + *
no need for this blank comment line
> */
> +
> static bool trap_debug_regs(struct kvm_vcpu *vcpu,
> const struct sys_reg_params *p,
> const struct sys_reg_desc *r)
> {
> - if (p->is_write) {
> - vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt);
> - vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> + if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW_BP) {
> + struct kvm_guest_debug_arch *saved;
> + __u64 *val;
> +
> + saved = &vcpu_debug_saved_reg(vcpu, debug_regs);
Here we don't bother enforcing num_brps/wrps, which is fine by me.
> +
> + if (r->reg >= DBGBCR0_EL1 && r->reg <= DBGBCR15_EL1)
> + val = &saved->dbg_bcr[r->reg - DBGBCR0_EL1];
> + else if (r->reg >= DBGBVR0_EL1 && r->reg <= DBGBVR15_EL1)
> + val = &saved->dbg_bvr[r->reg - DBGBVR0_EL1];
> + else if (r->reg >= DBGWCR0_EL1 && r->reg <= DBGWCR15_EL1)
> + val = &saved->dbg_wcr[r->reg - DBGWCR0_EL1];
> + else if (r->reg >= DBGWVR0_EL1 && r->reg <= DBGWVR15_EL1)
> + val = &saved->dbg_wvr[r->reg - DBGWVR0_EL1];
> + else {
> + kvm_err("Bad register index %d\n", r->reg);
> + return false;
> + }
> +
> + if (p->is_write)
> + *val = *vcpu_reg(vcpu, p->Rt);
> + else
> + *vcpu_reg(vcpu, p->Rt) = *val;
> +
> } else {
> - *vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg);
> + if (p->is_write) {
> + vcpu_sys_reg(vcpu, r->reg) = *vcpu_reg(vcpu, p->Rt);
> + vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
> + } else {
> + *vcpu_reg(vcpu, p->Rt) = vcpu_sys_reg(vcpu, r->reg);
> + }
> }
>
> return true;
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index ce2db14..0e48c0d 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -771,6 +771,8 @@ struct kvm_ppc_smmu_info {
> #define KVM_CAP_PPC_ENABLE_HCALL 104
> #define KVM_CAP_CHECK_EXTENSION_VM 105
> #define KVM_CAP_S390_USER_SIGP 106
> +#define KVM_CAP_GUEST_DEBUG_HW_BPS 107
> +#define KVM_CAP_GUEST_DEBUG_HW_WPS 108
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> --
> 2.3.4
>
^ permalink raw reply
* Re: [PATCH v4 04/10] eeprom: Add a simple EEPROM framework for eeprom consumers
From: Maxime Ripard @ 2015-04-10 11:45 UTC (permalink / raw)
To: Stephen Boyd
Cc: Srinivas Kandagatla, linux-arm-kernel, Rob Herring, Kumar Gala,
Mark Brown, s.hauer, Greg Kroah-Hartman, linux-api, linux-kernel,
devicetree, linux-arm-msm, arnd
In-Reply-To: <20150409144522.GB9663@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 3373 bytes --]
Hi Stephen,
On Thu, Apr 09, 2015 at 07:45:22AM -0700, Stephen Boyd wrote:
> > eeprom block array is just another way intended to get hold of
> > eeprom content for non-DT providers/consumers, but DT
> > consumers/providers can also use it. As of today SOC/mach level code
> > could use it as well.
> >
> > In eeprom_cell_get() case the lookup of provider is done based on
> > provider name, this provider name is generally supplied by all the
> > providers (both DT/non DT).
> >
> > for example in qfprom case,
> > provider is registered from DT with eeprom config containing a unique name:
> > static struct eeprom_config econfig = {
> > .name = "qfprom",
> > .id = 0,
> > };
> >
> > In the consumer case, the tsens driver could do some like in non DT way:
> >
> > struct eeprom_block blocks[4] ={
> > {
> > .offset = 0x404,
> > .count = 0x4,
> > },
> > {
> > .offset = 0x408,
> > .count = 0x4,
> > },
> > {
> > .offset = 0x40c,
> > .count = 0x4,
> > },
> > {
> > .offset = 0x410,
> > .count = 0x4,
> > },
> > };
> > calib_cell = eeprom_cell_get("qfprom0", blocks, 4);
> >
> >
> > Or in DT way
> > calib_cell = of_eeprom_cell_get(np, "calib");
> >
>
> Ok. I guess I was hoping for a more device centric approach like
> we have for clks/regulators/etc. That way a driver doesn't need
> to know it's using DT or not to figure out which API to call.
> Also the global namespace is sort of worrying (qfprom0 part). How
> do we know it's qfprom0? What if it's qfprom1 sometimes?
Through platform_data?
Or maybe that could be done using a function that would attach a cell
to a struct device?
I'm not sure that would cover all cases though.
> Also, how are we supposed to encode certain bits inside the
> stream of bytes (blocks)? In some situations (e.g. the qcom CPR
> stuff) we have quite a few fuse bits we need to read that are
> packed into different bytes and may cross byte boundaries (i.e.
> bits 6 to 10 of a 32-bit word). The current API looks to be byte
> focused when I have bit based/non-byte aligned data.
>
> My current feeling is that I don't want to use any of the block
> stuff at all. I just want a simple byte-based API that allows me
> to read a number of contiguous bytes from the fuses. Then I'll
> need to shift that data down by a few bits and mask it with the
> width of the data I want to read to extract the data needed.
Except that this is highly dependent on the usage you're making of
this API.
It could be fine to expect that if your writing machine code, that for
one given SoC will always be true.
If you're writing driver code, that will just have to lookup one value
from an EEPROM, the exact offset, size and device being completely
board dependant, it's not really practical.
> The only thing after that where I have a problem is figuring out
> which eeprom is present in the consumer driver.
Is that really necessary? I mean, the point of having an abstraction
layer is precisely to *not* care about the provider when you are the
consumer.
If we can't, then the abstraction layer is not good enough and we
should improve it, but we shouldn't need to hack around it.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] kdbus: uapi: Fix kernel-doc for enum kdbus_send_flags
From: Greg Kroah-Hartman @ 2015-04-10 11:44 UTC (permalink / raw)
To: Sergei Zviagintsev
Cc: Daniel Mack, David Herrmann, Djalal Harouni,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1428574261-18624-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org>
On Thu, Apr 09, 2015 at 01:11:01PM +0300, Sergei Zviagintsev wrote:
> Signed-off-by: Sergei Zviagintsev <sergei-E844GTqJAzo@public.gmane.org>
> Acked-by: Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>
> ---
> include/uapi/linux/kdbus.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks.
greg k-h
^ permalink raw reply
* [PATCH v4 8/8] macvtap/tun: add VNET_BE flag
From: Greg Kurz @ 2015-04-10 10:20 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150410101500.31843.61248.stgit@bahia.local>
The VNET_LE flag was introduced to fix accesses to virtio 1.0 headers
that are always little-endian. It can also be used to handle the special
case of a legacy little-endian device implemented by a big-endian host.
Let's add a flag and ioctls for big-endian devices as well. If both flags
are set, little-endian wins.
Since this is isn't a common usecase, the feature is controlled by a kernel
config option (not set by default).
Both macvtap and tun are covered by this patch since they share the same
API with userland.
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
drivers/net/Kconfig | 12 ++++++++
drivers/net/macvtap.c | 60 +++++++++++++++++++++++++++++++++++++++++-
drivers/net/tun.c | 62 ++++++++++++++++++++++++++++++++++++++++++-
include/uapi/linux/if_tun.h | 2 +
4 files changed, 134 insertions(+), 2 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index df51d60..f0e23a0 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -244,6 +244,18 @@ config TUN
If you don't know what to use this for, you don't need it.
+config TUN_VNET_BE
+ bool "Support for big-endian vnet headers"
+ default n
+ ---help---
+ This option allows TUN/TAP and MACVTAP device drivers to parse
+ vnet headers that are in big-endian byte order. It is useful
+ when the headers come from a big-endian legacy virtio driver and
+ the host is little-endian.
+
+ Unless you have a little-endian system hosting a big-endian virtual
+ machine with a virtio NIC, you should say N.
+
config VETH
tristate "Virtual ethernet pair device"
---help---
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 0a03a66..e0ab1b7 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -48,12 +48,27 @@ struct macvtap_queue {
#define MACVTAP_FEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE)
#define MACVTAP_VNET_LE 0x80000000
+#define MACVTAP_VNET_BE 0x40000000
+
+#ifdef CONFIG_TUN_VNET_BE
+static inline bool macvtap_legacy_is_little_endian(struct macvtap_queue *q)
+{
+ if (q->flags & MACVTAP_VNET_BE)
+ return false;
+ return virtio_legacy_is_little_endian();
+}
+#else
+static inline bool macvtap_legacy_is_little_endian(struct macvtap_queue *q)
+{
+ return virtio_legacy_is_little_endian();
+}
+#endif
static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
{
if (q->flags & MACVTAP_VNET_LE)
return true;
- return virtio_legacy_is_little_endian();
+ return macvtap_legacy_is_little_endian(q);
}
static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
@@ -1000,6 +1015,43 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
return 0;
}
+#ifdef CONFIG_TUN_VNET_BE
+static long macvtap_get_vnet_be(struct macvtap_queue *q, int __user *sp)
+{
+ int s = !!(q->flags & MACVTAP_VNET_BE);
+
+ if (put_user(s, sp))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long macvtap_set_vnet_be(struct macvtap_queue *q, int __user *sp)
+{
+ int s;
+
+ if (get_user(s, sp))
+ return -EFAULT;
+
+ if (s)
+ q->flags |= MACVTAP_VNET_BE;
+ else
+ q->flags &= ~MACVTAP_VNET_BE;
+
+ return 0;
+}
+#else
+static long macvtap_get_vnet_be(struct macvtap_queue *q, int __user *argp)
+{
+ return -EINVAL;
+}
+
+static long macvtap_set_vnet_be(struct macvtap_queue *q, int __user *argp)
+{
+ return -EINVAL;
+}
+#endif /* CONFIG_TUN_VNET_BE */
+
/*
* provide compatibility with generic tun/tap interface
*/
@@ -1097,6 +1149,12 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
q->flags &= ~MACVTAP_VNET_LE;
return 0;
+ case TUNGETVNETBE:
+ return macvtap_get_vnet_be(q, sp);
+
+ case TUNSETVNETBE:
+ return macvtap_set_vnet_be(q, sp);
+
case TUNSETOFFLOAD:
/* let the user check for future flags */
if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 053f9b6..4e12488 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -111,6 +111,7 @@ do { \
#define TUN_FASYNC IFF_ATTACH_QUEUE
/* High bits in flags field are unused. */
#define TUN_VNET_LE 0x80000000
+#define TUN_VNET_BE 0x40000000
#define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
IFF_MULTI_QUEUE)
@@ -206,11 +207,25 @@ struct tun_struct {
u32 flow_count;
};
+#ifdef CONFIG_TUN_VNET_BE
+static inline bool tun_legacy_is_little_endian(struct tun_struct *tun)
+{
+ if (tun->flags & TUN_VNET_BE)
+ return false;
+ return virtio_legacy_is_little_endian();
+}
+#else
+static inline bool tun_legacy_is_little_endian(struct tun_struct *tun)
+{
+ return virtio_legacy_is_little_endian();
+}
+#endif
+
static inline bool tun_is_little_endian(struct tun_struct *tun)
{
if (tun->flags & TUN_VNET_LE)
return true;
- return virtio_legacy_is_little_endian();
+ return tun_legacy_is_little_endian(tun);
}
static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
@@ -1836,6 +1851,43 @@ unlock:
return ret;
}
+#ifdef CONFIG_TUN_VNET_BE
+static long tun_get_vnet_be(struct tun_struct *tun, int __user *argp)
+{
+ int be = !!(tun->flags & TUN_VNET_BE);
+
+ if (put_user(be, argp))
+ return EFAULT;
+
+ return 0;
+}
+
+static long tun_set_vnet_be(struct tun_struct *tun, int __user *argp)
+{
+ int be;
+
+ if (get_user(be, argp))
+ return -EFAULT;
+
+ if (be)
+ tun->flags |= TUN_VNET_BE;
+ else
+ tun->flags &= ~TUN_VNET_BE;
+
+ return 0;
+}
+#else
+static long tun_get_vnet_be(struct tun_struct *tun, int __user *argp)
+{
+ return -EINVAL;
+}
+
+static long tun_set_vnet_be(struct tun_struct *tun, int __user *argp)
+{
+ return -EINVAL;
+}
+#endif /* CONFIG_TUN_VNET_BE */
+
static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
unsigned long arg, int ifreq_len)
{
@@ -2065,6 +2117,14 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
tun->flags &= ~TUN_VNET_LE;
break;
+ case TUNGETVNETBE:
+ ret = tun_get_vnet_be(tun, argp);
+ break;
+
+ case TUNSETVNETBE:
+ ret = tun_set_vnet_be(tun, argp);
+ break;
+
case TUNATTACHFILTER:
/* Can be set only for TAPs */
ret = -EINVAL;
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index 50ae243..bcac4c0 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -50,6 +50,8 @@
#define TUNGETFILTER _IOR('T', 219, struct sock_fprog)
#define TUNSETVNETLE _IOW('T', 220, int)
#define TUNGETVNETLE _IOR('T', 221, int)
+#define TUNSETVNETBE _IOW('T', 222, int)
+#define TUNGETVNETBE _IOR('T', 223, int)
/* TUNSETIFF ifr flags */
#define IFF_TUN 0x0001
^ permalink raw reply related
* [PATCH v4 7/8] vhost: feature to set the vring endianness
From: Greg Kurz @ 2015-04-10 10:19 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150410101500.31843.61248.stgit@bahia.local>
This patch brings cross-endian support to vhost when used to implement
legacy virtio devices. Since it is a relatively rare situation, the
feature availability is controlled by a kernel config option (not set
by default).
The vq->is_le boolean field is added to cache the endianness to be
used for ring accesses. It defaults to native endian, as expected
by legacy virtio devices. When the ring gets active, we force little
endian if the device is modern. When the ring is deactivated, we
revert to the native endian default.
If cross-endian was compiled in, a vq->user_be boolean field is added
so that userspace may request a specific endianness. This field is
used to override the default when activating the ring of a legacy
device. It has no effect on modern devices.
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
drivers/vhost/Kconfig | 10 ++++++
drivers/vhost/vhost.c | 76 +++++++++++++++++++++++++++++++++++++++++++-
drivers/vhost/vhost.h | 12 +++++--
include/uapi/linux/vhost.h | 9 +++++
4 files changed, 103 insertions(+), 4 deletions(-)
Changes since v3:
- VHOST_SET_VRING_ENDIAN_LEGACY ioctl renamed to VHOST_SET_VRING_BIG_ENDIAN
- ioctl API is now: 0 for le, 1 for be, other values are EINVAL
- ioctl doesn't filter out modern devices
- ioctl stubs return ENOIOCTLCMD
- forbid endianness changes when vring is active
- logic now handled with vq->is_le and vq->user_be according to device
start/stop as suggested by Michael
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index 017a1e8..0aec88c 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -32,3 +32,13 @@ config VHOST
---help---
This option is selected by any driver which needs to access
the core of vhost.
+
+config VHOST_SET_ENDIAN_LEGACY
+ bool "Cross-endian support for host kernel accelerator"
+ default n
+ ---help---
+ This option allows vhost to support guests with a different byte
+ ordering from host. It is disabled by default since it adds overhead
+ and it is only needed by a few platforms (powerpc and arm).
+
+ If unsure, say "N".
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2ee2826..3eb756b 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -199,6 +199,10 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call = NULL;
vq->log_ctx = NULL;
vq->memory = NULL;
+ vq->is_le = virtio_legacy_is_little_endian();
+#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY
+ vq->user_be = !vq->is_le;
+#endif
}
static int vhost_worker(void *data)
@@ -630,6 +634,53 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
return 0;
}
+#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY
+static long vhost_set_vring_big_endian(struct vhost_virtqueue *vq,
+ int __user *argp)
+{
+ struct vhost_vring_state s;
+
+ if (vq->private_data)
+ return -EBUSY;
+
+ if (copy_from_user(&s, argp, sizeof(s)))
+ return -EFAULT;
+
+ if (s.num && s.num != 1)
+ return -EINVAL;
+
+ vq->user_be = s.num;
+
+ return 0;
+}
+
+static long vhost_get_vring_big_endian(struct vhost_virtqueue *vq, u32 idx,
+ int __user *argp)
+{
+ struct vhost_vring_state s = {
+ .index = idx,
+ .num = vq->user_be
+ };
+
+ if (copy_to_user(argp, &s, sizeof(s)))
+ return -EFAULT;
+
+ return 0;
+}
+#else
+static long vhost_set_vring_big_endian(struct vhost_virtqueue *vq,
+ int __user *argp)
+{
+ return -ENOIOCTLCMD;
+}
+
+static long vhost_get_vring_big_endian(struct vhost_virtqueue *vq, u32 idx,
+ int __user *argp)
+{
+ return -ENOIOCTLCMD;
+}
+#endif /* CONFIG_VHOST_SET_ENDIAN_LEGACY */
+
long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
{
struct file *eventfp, *filep = NULL;
@@ -806,6 +857,12 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
} else
filep = eventfp;
break;
+ case VHOST_SET_VRING_BIG_ENDIAN:
+ r = vhost_set_vring_big_endian(vq, argp);
+ break;
+ case VHOST_GET_VRING_BIG_ENDIAN:
+ r = vhost_get_vring_big_endian(vq, idx, argp);
+ break;
default:
r = -ENOIOCTLCMD;
}
@@ -1040,12 +1097,29 @@ static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event)
return 0;
}
+#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY
+static void vhost_init_is_le(struct vhost_virtqueue *vq)
+{
+ vq->is_le = vhost_has_feature(vq, VIRTIO_F_VERSION_1) || !vq->user_be;
+}
+#else
+static void vhost_init_is_le(struct vhost_virtqueue *vq)
+{
+ if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
+ vq->is_le = true;
+}
+#endif
+
int vhost_init_used(struct vhost_virtqueue *vq)
{
__virtio16 last_used_idx;
int r;
- if (!vq->private_data)
+ if (!vq->private_data) {
+ vq->is_le = virtio_legacy_is_little_endian();
return 0;
+ }
+
+ vhost_init_is_le(vq);
r = vhost_update_used_flags(vq);
if (r)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 4e9a186..04b2add 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -106,6 +106,14 @@ struct vhost_virtqueue {
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
+
+ /* Ring endianness. Defaults to legacy native endianness.
+ * Set to true when starting a modern virtio device. */
+ bool is_le;
+#ifdef CONFIG_VHOST_SET_ENDIAN_LEGACY
+ /* Ring endianness requested by userspace for cross-endian support. */
+ bool user_be;
+#endif
};
struct vhost_dev {
@@ -175,9 +183,7 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
{
- if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
- return true;
- return virtio_legacy_is_little_endian();
+ return vq->is_le;
}
/* Memory accessors */
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index bb6a5b4..5cdebbc 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -103,6 +103,15 @@ struct vhost_memory {
/* Get accessor: reads index, writes value in num */
#define VHOST_GET_VRING_BASE _IOWR(VHOST_VIRTIO, 0x12, struct vhost_vring_state)
+/* Set the vring byte order in num. This is a legacy only API that is simply
+ * ignored when VIRTIO_F_VERSION_1 is set.
+ * 0 to set to little-endian
+ * 1 to set to big-endian
+ * other values return EINVAL.
+ */
+#define VHOST_SET_VRING_BIG_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct vhost_vring_state)
+#define VHOST_GET_VRING_BIG_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct vhost_vring_state)
+
/* The following ioctls use eventfd file descriptors to signal and poll
* for events. */
^ permalink raw reply related
* [PATCH v4 6/8] virtio: add explicit big-endian support to memory accessors
From: Greg Kurz @ 2015-04-10 10:16 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: Cornelia Huck, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, kvm-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
In-Reply-To: <20150410101500.31843.61248.stgit-GiB8zCg7hOfDOqzlkpFKJg@public.gmane.org>
The current memory accessors logic is:
- little endian if little_endian
- native endian (i.e. no byteswap) if !little_endian
If we want to fully support cross-endian vhost, we also need to be
able to convert to big endian.
Instead of changing the little_endian argument to some 3-value enum, this
patch changes the logic to:
- little endian if little_endian
- big endian if !little_endian
The native endian case is handled by all users with a trivial helper. This
patch doesn't change any functionality, nor it does add overhead.
Signed-off-by: Greg Kurz <gkurz-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
drivers/net/macvtap.c | 4 +++-
drivers/net/tun.c | 4 +++-
drivers/vhost/vhost.h | 4 +++-
include/linux/virtio_byteorder.h | 24 ++++++++++++++----------
include/linux/virtio_config.h | 4 +++-
include/linux/vringh.h | 4 +++-
6 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index a2f2958..0a03a66 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -51,7 +51,9 @@ struct macvtap_queue {
static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
{
- return q->flags & MACVTAP_VNET_LE;
+ if (q->flags & MACVTAP_VNET_LE)
+ return true;
+ return virtio_legacy_is_little_endian();
}
static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3c3d6c0..053f9b6 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -208,7 +208,9 @@ struct tun_struct {
static inline bool tun_is_little_endian(struct tun_struct *tun)
{
- return tun->flags & TUN_VNET_LE;
+ if (tun->flags & TUN_VNET_LE)
+ return true;
+ return virtio_legacy_is_little_endian();
}
static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 6a49960..4e9a186 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -175,7 +175,9 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
{
- return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+ if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
+ return true;
+ return virtio_legacy_is_little_endian();
}
/* Memory accessors */
diff --git a/include/linux/virtio_byteorder.h b/include/linux/virtio_byteorder.h
index 51865d0..ce63a2c 100644
--- a/include/linux/virtio_byteorder.h
+++ b/include/linux/virtio_byteorder.h
@@ -3,17 +3,21 @@
#include <linux/types.h>
#include <uapi/linux/virtio_types.h>
-/*
- * Low-level memory accessors for handling virtio in modern little endian and in
- * compatibility native endian format.
- */
+static inline bool virtio_legacy_is_little_endian(void)
+{
+#ifdef __LITTLE_ENDIAN
+ return true;
+#else
+ return false;
+#endif
+}
static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val)
{
if (little_endian)
return le16_to_cpu((__force __le16)val);
else
- return (__force u16)val;
+ return be16_to_cpu((__force __be16)val);
}
static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)
@@ -21,7 +25,7 @@ static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)
if (little_endian)
return (__force __virtio16)cpu_to_le16(val);
else
- return (__force __virtio16)val;
+ return (__force __virtio16)cpu_to_be16(val);
}
static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)
@@ -29,7 +33,7 @@ static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)
if (little_endian)
return le32_to_cpu((__force __le32)val);
else
- return (__force u32)val;
+ return be32_to_cpu((__force __be32)val);
}
static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)
@@ -37,7 +41,7 @@ static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)
if (little_endian)
return (__force __virtio32)cpu_to_le32(val);
else
- return (__force __virtio32)val;
+ return (__force __virtio32)cpu_to_be32(val);
}
static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)
@@ -45,7 +49,7 @@ static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)
if (little_endian)
return le64_to_cpu((__force __le64)val);
else
- return (__force u64)val;
+ return be64_to_cpu((__force __be64)val);
}
static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val)
@@ -53,7 +57,7 @@ static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val)
if (little_endian)
return (__force __virtio64)cpu_to_le64(val);
else
- return (__force __virtio64)val;
+ return (__force __virtio64)cpu_to_be64(val);
}
#endif /* _LINUX_VIRTIO_BYTEORDER */
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index bd1a582..36a6daa 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -207,7 +207,9 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
static inline bool virtio_is_little_endian(struct virtio_device *vdev)
{
- return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+ if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
+ return true;
+ return virtio_legacy_is_little_endian();
}
/* Memory accessors */
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index 3ed62ef..d786c2d 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -228,7 +228,9 @@ static inline void vringh_notify(struct vringh *vrh)
static inline bool vringh_is_little_endian(const struct vringh *vrh)
{
- return vrh->little_endian;
+ if (vrh->little_endian)
+ return true;
+ return virtio_legacy_is_little_endian();
}
static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val)
^ permalink raw reply related
* [PATCH v4 5/8] vhost: introduce vhost_is_little_endian() helper
From: Greg Kurz @ 2015-04-10 10:16 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150410101500.31843.61248.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
drivers/vhost/vhost.h | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8c1c792..6a49960 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -173,34 +173,39 @@ static inline bool vhost_has_feature(struct vhost_virtqueue *vq, int bit)
return vq->acked_features & (1ULL << bit);
}
+static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
+{
+ return vhost_has_feature(vq, VIRTIO_F_VERSION_1);
+}
+
/* Memory accessors */
static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val)
{
- return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+ return __virtio16_to_cpu(vhost_is_little_endian(vq), val);
}
static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val)
{
- return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio16(vhost_is_little_endian(vq), val);
}
static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val)
{
- return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+ return __virtio32_to_cpu(vhost_is_little_endian(vq), val);
}
static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val)
{
- return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio32(vhost_is_little_endian(vq), val);
}
static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val)
{
- return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+ return __virtio64_to_cpu(vhost_is_little_endian(vq), val);
}
static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val)
{
- return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio64(vhost_is_little_endian(vq), val);
}
#endif
^ permalink raw reply related
* [PATCH v4 4/8] vringh: introduce vringh_is_little_endian() helper
From: Greg Kurz @ 2015-04-10 10:15 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150410101500.31843.61248.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
include/linux/vringh.h | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index a3fa537..3ed62ef 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -226,33 +226,38 @@ static inline void vringh_notify(struct vringh *vrh)
vrh->notify(vrh);
}
+static inline bool vringh_is_little_endian(const struct vringh *vrh)
+{
+ return vrh->little_endian;
+}
+
static inline u16 vringh16_to_cpu(const struct vringh *vrh, __virtio16 val)
{
- return __virtio16_to_cpu(vrh->little_endian, val);
+ return __virtio16_to_cpu(vringh_is_little_endian(vrh), val);
}
static inline __virtio16 cpu_to_vringh16(const struct vringh *vrh, u16 val)
{
- return __cpu_to_virtio16(vrh->little_endian, val);
+ return __cpu_to_virtio16(vringh_is_little_endian(vrh), val);
}
static inline u32 vringh32_to_cpu(const struct vringh *vrh, __virtio32 val)
{
- return __virtio32_to_cpu(vrh->little_endian, val);
+ return __virtio32_to_cpu(vringh_is_little_endian(vrh), val);
}
static inline __virtio32 cpu_to_vringh32(const struct vringh *vrh, u32 val)
{
- return __cpu_to_virtio32(vrh->little_endian, val);
+ return __cpu_to_virtio32(vringh_is_little_endian(vrh), val);
}
static inline u64 vringh64_to_cpu(const struct vringh *vrh, __virtio64 val)
{
- return __virtio64_to_cpu(vrh->little_endian, val);
+ return __virtio64_to_cpu(vringh_is_little_endian(vrh), val);
}
static inline __virtio64 cpu_to_vringh64(const struct vringh *vrh, u64 val)
{
- return __cpu_to_virtio64(vrh->little_endian, val);
+ return __cpu_to_virtio64(vringh_is_little_endian(vrh), val);
}
#endif /* _LINUX_VRINGH_H */
^ permalink raw reply related
* [PATCH v4 3/8] macvtap: introduce macvtap_is_little_endian() helper
From: Greg Kurz @ 2015-04-10 10:15 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150410101500.31843.61248.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
drivers/net/macvtap.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 27ecc5c..a2f2958 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -49,14 +49,19 @@ struct macvtap_queue {
#define MACVTAP_VNET_LE 0x80000000
+static inline bool macvtap_is_little_endian(struct macvtap_queue *q)
+{
+ return q->flags & MACVTAP_VNET_LE;
+}
+
static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val)
{
- return __virtio16_to_cpu(q->flags & MACVTAP_VNET_LE, val);
+ return __virtio16_to_cpu(macvtap_is_little_endian(q), val);
}
static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val)
{
- return __cpu_to_virtio16(q->flags & MACVTAP_VNET_LE, val);
+ return __cpu_to_virtio16(macvtap_is_little_endian(q), val);
}
static struct proto macvtap_proto = {
^ permalink raw reply related
* [PATCH v4 2/8] tun: add tun_is_little_endian() helper
From: Greg Kurz @ 2015-04-10 10:15 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150410101500.31843.61248.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
drivers/net/tun.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 857dca4..3c3d6c0 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -206,14 +206,19 @@ struct tun_struct {
u32 flow_count;
};
+static inline bool tun_is_little_endian(struct tun_struct *tun)
+{
+ return tun->flags & TUN_VNET_LE;
+}
+
static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val)
{
- return __virtio16_to_cpu(tun->flags & TUN_VNET_LE, val);
+ return __virtio16_to_cpu(tun_is_little_endian(tun), val);
}
static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val)
{
- return __cpu_to_virtio16(tun->flags & TUN_VNET_LE, val);
+ return __cpu_to_virtio16(tun_is_little_endian(tun), val);
}
static inline u32 tun_hashfn(u32 rxhash)
^ permalink raw reply related
* [PATCH v4 1/8] virtio: introduce virtio_is_little_endian() helper
From: Greg Kurz @ 2015-04-10 10:15 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
In-Reply-To: <20150410101500.31843.61248.stgit@bahia.local>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
include/linux/virtio_config.h | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index ca3ed78..bd1a582 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -205,35 +205,40 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu)
return 0;
}
+static inline bool virtio_is_little_endian(struct virtio_device *vdev)
+{
+ return virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
+}
+
/* Memory accessors */
static inline u16 virtio16_to_cpu(struct virtio_device *vdev, __virtio16 val)
{
- return __virtio16_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio16_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio16 cpu_to_virtio16(struct virtio_device *vdev, u16 val)
{
- return __cpu_to_virtio16(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio16(virtio_is_little_endian(vdev), val);
}
static inline u32 virtio32_to_cpu(struct virtio_device *vdev, __virtio32 val)
{
- return __virtio32_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio32_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio32 cpu_to_virtio32(struct virtio_device *vdev, u32 val)
{
- return __cpu_to_virtio32(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio32(virtio_is_little_endian(vdev), val);
}
static inline u64 virtio64_to_cpu(struct virtio_device *vdev, __virtio64 val)
{
- return __virtio64_to_cpu(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __virtio64_to_cpu(virtio_is_little_endian(vdev), val);
}
static inline __virtio64 cpu_to_virtio64(struct virtio_device *vdev, u64 val)
{
- return __cpu_to_virtio64(virtio_has_feature(vdev, VIRTIO_F_VERSION_1), val);
+ return __cpu_to_virtio64(virtio_is_little_endian(vdev), val);
}
/* Config space accessors. */
^ permalink raw reply related
* [PATCH v4 0/8] vhost: support for cross endian guests
From: Greg Kurz @ 2015-04-10 10:15 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin
Cc: linux-api, linux-kernel, kvm, virtualization
Hi,
This patchset allows vhost to be used with legacy virtio when guest and host
have a different endianness.
Patch 7 got rewritten according to Cornelia's and Michael's comments. I have
also introduced patch 8 that brings BE vnet headers support to tun/macvtap.
This series is enough to have vhost_net working flawlessly. I could
succesfully reboot guests from ppc64 to ppc64le and vice-versa on ppc64
and ppc64le hosts.
---
Greg Kurz (8):
virtio: introduce virtio_is_little_endian() helper
tun: add tun_is_little_endian() helper
macvtap: introduce macvtap_is_little_endian() helper
vringh: introduce vringh_is_little_endian() helper
vhost: introduce vhost_is_little_endian() helper
virtio: add explicit big-endian support to memory accessors
vhost: feature to set the vring endianness
macvtap/tun: add VNET_BE flag
drivers/net/Kconfig | 12 ++++++
drivers/net/macvtap.c | 69 ++++++++++++++++++++++++++++++++++-
drivers/net/tun.c | 71 +++++++++++++++++++++++++++++++++++-
drivers/vhost/Kconfig | 10 +++++
drivers/vhost/vhost.c | 76 ++++++++++++++++++++++++++++++++++++++
drivers/vhost/vhost.h | 25 ++++++++++---
include/linux/virtio_byteorder.h | 24 +++++++-----
include/linux/virtio_config.h | 19 +++++++---
include/linux/vringh.h | 19 +++++++---
include/uapi/linux/if_tun.h | 2 +
include/uapi/linux/vhost.h | 9 +++++
11 files changed, 303 insertions(+), 33 deletions(-)
--
Greg
^ permalink raw reply
* Re: [PATCH] audit.h: remove the macro AUDIT_ARCH_ARMEB definition
From: Li RongQing @ 2015-04-10 0:21 UTC (permalink / raw)
To: Steve Grubb
Cc: linux-audit-H+wXaHxf7aLQT0dZR+AlfA, Paul Moore,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <18079199.sKrYvVCScd@x2>
On Fri, Apr 3, 2015 at 8:42 PM, Steve Grubb <sgrubb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Since this #define lives in the user visible headers I don't want to
>> remove it and risk causing a userspace breakage. Leaving the #define
>> in the header, even if it is unused by modern userspace, is harmless.
>
> This macro doesn't appear to be in use by auditd.
ping ...
^ permalink raw reply
* Re: [patch -mm] mm, doc: cleanup and clarify munmap behavior for hugetlb memory fix
From: David Rientjes @ 2015-04-09 19:46 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Andrew Morton, Hugh Dickins, Davide Libenzi, Luiz Capitulino,
Shuah Khan, Andrea Arcangeli, Joern Engel, Jianguo Wu,
Eric B Munson, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150404113456.55468dc3-T1hC0tSOHrs@public.gmane.org>
On Sat, 4 Apr 2015, Jonathan Corbet wrote:
> On Thu, 2 Apr 2015 15:50:15 -0700 (PDT)
> David Rientjes <rientjes-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>
> > Don't only specify munmap(2) behavior with respect the hugetlb memory, all
> > other syscalls get naturally aligned to the native page size of the
> > processor. Rather, pick out munmap(2) as a specific example.
>
> So I was going to apply this to the docs tree, but it doesn't even come
> close. What tree was this patch generated against?
>
Sorry, it's not intended to go through the docs tree, it's a patch to fix
mm-doc-cleanup-and-clarify-munmap-behavior-for-hugetlb-memory.patch in
-mm. It's been merged into that tree, but I would still appreciate your
ack!
^ permalink raw reply
* [PATCH v3 4/4] Documentation: ABI: Fix documentation for mass_storage function
From: Krzysztof Opasiak @ 2015-04-09 16:18 UTC (permalink / raw)
To: balbi-l0cyMroinI0, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
jlbec-aKy9MeLSZ9dg9hUCZPvPmw
Cc: andrzej.p-Sze3O3UU22JBDgjK7y7TUQ,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Krzysztof Opasiak
In-Reply-To: <1428596290-23637-1-git-send-email-k.opasiak-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Luns in mass storage function are identified using their id.
While creating lun's directory user cannot choose any arbitrary
name other than decimal value from 1 to FSG_MAX_LUNS.
Moreover, LUNs ids should be contiguous. This means that user
may remove only lun with max id and can create new lun
only if its id equals to max id + 1.
Signed-off-by: Krzysztof Opasiak <k.opasiak-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
.../ABI/testing/configfs-usb-gadget-mass-storage | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-mass-storage b/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
index 9931fb0..2bf085d 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
+++ b/Documentation/ABI/testing/configfs-usb-gadget-mass-storage
@@ -11,10 +11,15 @@ Description:
are 2..4. Available only if
CONFIG_USB_GADGET_DEBUG_FILES is set.
-What: /config/usb-gadget/gadget/functions/mass_storage.name/lun.name
+What: /config/usb-gadget/gadget/functions/mass_storage.name/lun.id
Date: Oct 2013
KernelVersion: 3.13
Description:
+ id - decimal value from 1 to FSG_MAX_LUNS
+ (which is 8 by default) - 1. LUNs should be numbered contiguously.
+ lun.0 is reserved for default lun which appears while creating
+ mass_storage.name directory and cannot be removed by the user.
+
The attributes:
file - The path to the backing file for the LUN.
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 3/4] usb: gadget: mass_storage: Ensure that lun ids are contiguous
From: Krzysztof Opasiak @ 2015-04-09 16:18 UTC (permalink / raw)
To: balbi, gregkh, jlbec
Cc: andrzej.p, m.szyprowski, linux-api, linux-kernel, linux-usb,
Krzysztof Opasiak
In-Reply-To: <1428596290-23637-1-git-send-email-k.opasiak@samsung.com>
According to mass storage specification:
"Logical Unit Numbers on the device shall be numbered
contiguously starting from LUN 0 to a maximum LUN of 15 (Fh)."
This commit fix configfs interface adding this restriction.
Now user can create luns only with contignous ids and
cannot remove lun from the middle of id space.
Example:
as is:
$ mkdir mass_storage.name
$ mkdir lun.3
$ mkdir lun.5
$ rmdir lun.3
After this commit:
$ mkdir mass_storage.name
$ mkdir lun.3
mkdir: cannot create directory 'lun.3': Invalid argument
$ mkdir lun.1
$ mkdir lun.2
$ rmdir lun.1
rmdir: failed to remove 'lun.1': Device or resource busy
$ rmdir lun.2
$ rmdir lun.1
Signed-off-by: Krzysztof Opasiak <k.opasiak@samsung.com>
---
drivers/usb/gadget/function/f_mass_storage.c | 29 ++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 67a67b5..f4b2de4 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3355,6 +3355,12 @@ static struct config_group *fsg_lun_make(struct config_group *group,
goto out;
}
+ if (!fsg_opts->common->luns[num - 1]) {
+ ret = -EINVAL;
+ pr_err("LUN ids should be contiguous\n");
+ goto out;
+ }
+
opts = kzalloc(sizeof(*opts), GFP_KERNEL);
if (!opts) {
ret = -ENOMEM;
@@ -3364,12 +3370,17 @@ static struct config_group *fsg_lun_make(struct config_group *group,
memset(&config, 0, sizeof(config));
config.removable = true;
+ /* ensure that lun ids are contiguous */
+ ret = configfs_depend_item_unlocked(&(fsg_opts->lun_opts
+ [num - 1]->group.cg_item));
+ if (ret)
+ goto err_free_opts;
+
ret = fsg_common_create_lun(fsg_opts->common, &config, num, name,
(const char **)&group->cg_item.ci_name);
- if (ret) {
- kfree(opts);
- goto out;
- }
+ if (ret)
+ goto err_undepend_item;
+
opts->lun = fsg_opts->common->luns[num];
opts->lun_id = num;
WARN_ON(fsg_opts->lun_opts[num]);
@@ -3379,6 +3390,12 @@ static struct config_group *fsg_lun_make(struct config_group *group,
config_group_init_type_name(&opts->group, name, &fsg_lun_type);
return &opts->group;
+
+err_undepend_item:
+ configfs_undepend_item_unlocked(&(fsg_opts->lun_opts
+ [num - 1]->group.cg_item));
+err_free_opts:
+ kfree(opts);
out:
mutex_unlock(&fsg_opts->lock);
return ERR_PTR(ret);
@@ -3400,6 +3417,10 @@ static void fsg_lun_drop(struct config_group *group, struct config_item *item)
unregister_gadget_item(gadget);
}
+ /* Allow to remove next one */
+ configfs_undepend_item_unlocked(&(fsg_opts->lun_opts
+ [lun_opts->lun_id - 1]->group.cg_item));
+
fsg_common_remove_lun(lun_opts->lun, fsg_opts->common->sysfs);
fsg_opts->common->luns[lun_opts->lun_id] = NULL;
fsg_opts->lun_opts[lun_opts->lun_id] = NULL;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 2/4] usb: gadget: mass_storage: Store lun_opts in fsg_opts
From: Krzysztof Opasiak @ 2015-04-09 16:18 UTC (permalink / raw)
To: balbi, gregkh, jlbec
Cc: andrzej.p, m.szyprowski, linux-api, linux-kernel, linux-usb,
Krzysztof Opasiak
In-Reply-To: <1428596290-23637-1-git-send-email-k.opasiak@samsung.com>
Currently lun_opts are stored only in configfs and
accessed using container_of() on config group. This
means that in configfs callbacks we can easily access
only current lun (config_group).
This commit adds an additinal array to fsg_opts which
allows to access not only current lun but also all other
luns using their id.
Signed-off-by: Krzysztof Opasiak <k.opasiak@samsung.com>
---
drivers/usb/gadget/function/f_mass_storage.c | 5 +++++
drivers/usb/gadget/function/f_mass_storage.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 811929c..67a67b5 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -3372,6 +3372,8 @@ static struct config_group *fsg_lun_make(struct config_group *group,
}
opts->lun = fsg_opts->common->luns[num];
opts->lun_id = num;
+ WARN_ON(fsg_opts->lun_opts[num]);
+ fsg_opts->lun_opts[num] = opts;
mutex_unlock(&fsg_opts->lock);
config_group_init_type_name(&opts->group, name, &fsg_lun_type);
@@ -3400,6 +3402,7 @@ static void fsg_lun_drop(struct config_group *group, struct config_item *item)
fsg_common_remove_lun(lun_opts->lun, fsg_opts->common->sysfs);
fsg_opts->common->luns[lun_opts->lun_id] = NULL;
+ fsg_opts->lun_opts[lun_opts->lun_id] = NULL;
lun_opts->lun_id = 0;
mutex_unlock(&fsg_opts->lock);
@@ -3546,6 +3549,7 @@ static struct usb_function_instance *fsg_alloc_inst(void)
if (!opts)
return ERR_PTR(-ENOMEM);
mutex_init(&opts->lock);
+ memset(opts->lun_opts, 0, sizeof(opts->lun_opts));
opts->func_inst.free_func_inst = fsg_free_inst;
opts->common = fsg_common_setup(opts->common);
if (IS_ERR(opts->common)) {
@@ -3569,6 +3573,7 @@ static struct usb_function_instance *fsg_alloc_inst(void)
(const char **)&opts->func_inst.group.cg_item.ci_name);
opts->lun0.lun = opts->common->luns[0];
opts->lun0.lun_id = 0;
+ opts->lun_opts[0] = &opts->lun0;
config_group_init_type_name(&opts->lun0.group, "lun.0", &fsg_lun_type);
opts->default_groups[0] = &opts->lun0.group;
opts->func_inst.group.default_groups = opts->default_groups;
diff --git a/drivers/usb/gadget/function/f_mass_storage.h b/drivers/usb/gadget/function/f_mass_storage.h
index b4866fc..0a7c656 100644
--- a/drivers/usb/gadget/function/f_mass_storage.h
+++ b/drivers/usb/gadget/function/f_mass_storage.h
@@ -81,6 +81,7 @@ struct fsg_opts {
struct fsg_common *common;
struct usb_function_instance func_inst;
struct fsg_lun_opts lun0;
+ struct fsg_lun_opts *lun_opts[FSG_MAX_LUNS];
struct config_group *default_groups[2];
bool no_configfs; /* for legacy gadgets */
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox