Linux userland API discussions
 help / color / mirror / Atom feed
* 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),
>> +			       &regs->dbg_bcr,
>> +			       sizeof(__u64)*nb);
>> +			memcpy(&vcpu_sys_reg(vcpu, DBGBVR0_EL1),
>> +			       &regs->dbg_bvr,
>> +			       sizeof(__u64)*nb);
>> +			memcpy(&vcpu_sys_reg(vcpu, DBGWCR0_EL1),
>> +			       &regs->dbg_wcr,
>> +			       sizeof(__u64)*nw);
>> +			memcpy(&vcpu_sys_reg(vcpu, DBGWVR0_EL1),
>> +			       &regs->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: [PATCH v3 03/21] Drivers: hv: kvp: move poll_channel() to hyperv_vmbus.h
From: Vitaly Kuznetsov @ 2015-04-13  8:36 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: K. Y. Srinivasan, devel-tBiZLqfeLfOHmIFyCCdPziST3g8Odh+X,
	Radim Krcmar, Greg Kroah-Hartman, Haiyang Zhang,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150410182742.GQ10964@mwanda>

Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> writes:

> 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.
>

Oh, yea, I see... I'll try eliminating this practice for myself in
future, but it seems many other contributors never saw their commits
with '...' in a web archive as well..

> regards,
> dan carpenter

-- 
  Vitaly

^ permalink raw reply

* Re: [PATCH v2 01/10] KVM: add commentary for kvm_debug_exit_arch struct
From: Christoffer Dall @ 2015-04-13 10:57 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kvm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, marc.zyngier-5wv7dgnIgG8,
	peter.maydell-QSEj5FYQhm4dnm+yROfE0A, agraf-l3A5Bk7waGM,
	drjones-H+wXaHxf7aLQT0dZR+AlfA, pbonzini-H+wXaHxf7aLQT0dZR+AlfA,
	zhichao.huang-QSEj5FYQhm4dnm+yROfE0A,
	jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ,
	dahi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	r65777-KZfg59tc24xl57MIdRCFDg, bp-l3A5Bk7waGM, Gleb Natapov,
	open list:ABI/API, open list
In-Reply-To: <1427814488-28467-2-git-send-email-alex.bennee-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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'm fine with this change as it is, but I think it should update the
documenation of the kvm_run structure along with it, and in there it
seems the debug struct is listed as 'unused'; this should be addressed
in this patch set somewhere if we start using this struct?

-Christoffer

^ permalink raw reply

* Re: [PATCH v2 02/10] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
From: Christoffer Dall @ 2015-04-13 11:55 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,

can you be more specific than 'stuff' here?  features?

> + * however there is some shared commonality for the common cases

I don't like this sentence; shared commonality is a pleonasm and the use
of however makes it sounds like there's some caveat here.

If the top 16 bits are indeed arhictecture specific, then I think they
should just be defined in their architecture specific headers.  Unless
the idea here is that there's a fixed set of of flags that architectures
can choose to support, in which case it should simply be defined in the
common header.


> + */
> +#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
> 

^ permalink raw reply

* Re: [PATCH v2 02/10] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
From: Alex Bennée @ 2015-04-13 14:51 UTC (permalink / raw)
  To: Christoffer Dall
  Cc: peter.maydell, kvm, Alexey Kardashevskiy, Bharat Bhushan,
	Paul Mackerras, H. Peter Anvin, kvmarm, Nadav Amit,
	maintainer:X86 ARCHITECTURE..., agraf, Gleb Natapov, Ingo Molnar,
	zhichao.huang, jan.kiszka, Mihai Caraman, bp, drjones,
	marc.zyngier, r65777, Thomas Gleixner, linux-arm-kernel,
	open list:ABI/API, open list, dahi, pbonzini,
	open list:LINUX FOR POWERPC...
In-Reply-To: <20150413115535.GM6186@cbox>


Christoffer Dall <christoffer.dall@linaro.org> writes:

> 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,
>
> can you be more specific than 'stuff' here?  features?
>
>> + * however there is some shared commonality for the common cases
>
> I don't like this sentence; shared commonality is a pleonasm and the use
> of however makes it sounds like there's some caveat here.

OK I can see that - after I looked it up ;-)

> If the top 16 bits are indeed arhictecture specific, then I think they
> should just be defined in their architecture specific headers.  Unless
> the idea here is that there's a fixed set of of flags that architectures
> can choose to support, in which case it should simply be defined in the
> common header.

Well an architecture might not support some features and want to use
those bits for something else? I didn't want to force the bottom two
of the architecture specific bits to wasted if the features don't exist.

>
>
>> + */
>> +#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
>> 

-- 
Alex Bennée
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH v2 02/10] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
From: Andrew Jones @ 2015-04-13 15:07 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: <871tjokrnu.fsf@linaro.org>

On Mon, Apr 13, 2015 at 03:51:33PM +0100, Alex Bennée wrote:
> 
> Christoffer Dall <christoffer.dall@linaro.org> writes:
> 
> > 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,
> >
> > can you be more specific than 'stuff' here?  features?
> >
> >> + * however there is some shared commonality for the common cases
> >
> > I don't like this sentence; shared commonality is a pleonasm and the use
> > of however makes it sounds like there's some caveat here.
> 
> OK I can see that - after I looked it up ;-)
> 
> > If the top 16 bits are indeed arhictecture specific, then I think they
> > should just be defined in their architecture specific headers.  Unless
> > the idea here is that there's a fixed set of of flags that architectures
> > can choose to support, in which case it should simply be defined in the
> > common header.
> 
> Well an architecture might not support some features and want to use
> those bits for something else? I didn't want to force the bottom two
> of the architecture specific bits to wasted if the features don't exist.

This change comes from a discussion we had on v1 of this series
http://www.spinics.net/lists/kvm-arm/msg12409.html

> 
> >
> >
> >> + */
> >> +#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
> >> 
> 
> -- 
> Alex Bennée

^ permalink raw reply

* Re: [v12 2/5] ext4: adds project ID support
From: Li Xi @ 2015-04-13 16:33 UTC (permalink / raw)
  To: Andreas Dilger
  Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Ext4 Developers List,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Theodore Ts'o, Jan Kara,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org,
	hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, Dmitry Monakhov
In-Reply-To: <C5339F6B-2DAE-4984-86CF-B2990CEC3550-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org>

Sorry Andreas, I will update the patch soon.

On Fri, Apr 10, 2015 at 5:37 PM, Andreas Dilger <adilger-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org> wrote:
> 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

* [RFC/PATCH v2 0/5] Add live source objects to DRM
From: Laurent Pinchart @ 2015-04-13 18:39 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, linux-api, linux-sh, Magnus Damm, linux-media

Hello,

Here's a proposal for a different approach to live source in DRM based on an
idea by Daniel Vetter. The previous version can be found at
http://lists.freedesktop.org/archives/dri-devel/2015-March/079319.html.

The need comes from the Renesas R-Car SoCs in which a video processing engine
(named VSP1) that operates from memory to memory has one output directly
connected to a plane of the display engine (DU) without going through memory.

The VSP1 is supported by a V4L2 driver. While it could be argued that it
should instead be supported directly by the DRM rcar-du driver, this wouldn't
be a good idea for at least two reasons. First, the R-Car SoCs contain several
VSP1 instances, of which only a subset have a direct DU connection. The only
other instances operate solely in memory to memory mode. Then, the VSP1 is a
video processing engine and not a display engine. Its features are easily
supported by the V4L2 API, but don't map to the DRM/KMS API. Significant
changes to DRM/KMS would be required, beyond what is in my opinion an
acceptable scope for a display API.

Now that the need to interface two separate devices supported by two different
drivers in two separate subsystems has been established, we need an API to do
so. It should be noted that while that API doesn't exist in the mainline
kernel, the need isn't limited to Renesas SoCs.

This patch set proposes one possible solution for the problem in the form of a
new DRM object named live source. Live sources are created by drivers to model
hardware connections between a plane input and an external source, and are
attached to planes through the KMS userspace API.

Patch 1/5 adds live source objects to DRM, with an in-kernel API for drivers
to register the sources, and a userspace API to enumerate them.

Patch 2/5 implements connection between live sources and planes through
framebuffers. It introduces a new live source flag for framebuffers. When a
framebuffer is created with that flag set, a live source is associated with
the framebuffer instead of buffer objects. The framebuffer can then be used
with a plane to connect it with the live source. This is the biggest
difference compared to the previous approach, and has several benefits:

- Changes are less intrusive in the DRM core
- The implementation supports both the legacy API and atomic updates without
  any code specific to either
- No changes to existing drivers are needed
- The framebuffer format and size configuration API is reused

The framebuffer format and size should ideally be validated using information
queried directly from the driver that supports the live source device, but
I've decided not to implement such communication between V4L2 and DRM/KMS at
the moment to keep the proposal simple.

Patches 3/5 to 5/5 then implement support for live sources in the R-Car DU
driver. The rcar_du_live_framebuffer structure and its associated helper
functions could be moved to the DRM core later if other drivers need a similar
implementation. I've decided to keep them in the rcar-du driver for now as
it's not clear yet what other drivers might need.

Once again nothing here is set in stone.

Laurent Pinchart (5):
  drm: Add live source object
  drm: Connect live source to framebuffers
  drm/rcar-du: Add VSP1 support to the planes allocator
  drm/rcar-du: Add VSP1 live source support
  drm/rcar-du: Restart the DU group when a plane source changes

 drivers/gpu/drm/drm_crtc.c              | 287 +++++++++++++++++++++++++++++---
 drivers/gpu/drm/drm_ioctl.c             |   2 +
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  10 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c   |   6 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |   3 +
 drivers/gpu/drm/rcar-du/rcar_du_group.c |  21 ++-
 drivers/gpu/drm/rcar-du/rcar_du_group.h |   2 +
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 132 ++++++++++++++-
 drivers/gpu/drm/rcar-du/rcar_du_kms.h   |   3 +
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 191 ++++++++++++++++-----
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |  11 ++
 drivers/gpu/drm/rcar-du/rcar_du_regs.h  |   1 +
 include/drm/drm_crtc.h                  |  35 ++++
 include/uapi/drm/drm.h                  |   3 +
 include/uapi/drm/drm_mode.h             |  23 +++
 15 files changed, 647 insertions(+), 83 deletions(-)

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [RFC/PATCH v2 1/5] drm: Add live source object
From: Laurent Pinchart @ 2015-04-13 18:39 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Magnus Damm, Daniel Vetter
In-Reply-To: <1428950387-6913-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

Live sources represent a hardware connection between a video stream
source and a CRTC, going through a plane. The kernel API lets driver
register live sources, and the userspace API lets applications enumerate
them.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
 drivers/gpu/drm/drm_crtc.c  | 164 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_ioctl.c |   2 +
 include/drm/drm_crtc.h      |  35 ++++++++++
 include/uapi/drm/drm.h      |   3 +
 include/uapi/drm/drm_mode.h |  16 +++++
 5 files changed, 220 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b3989e23195e..1f71978b4f17 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1265,6 +1265,70 @@ void drm_plane_cleanup(struct drm_plane *plane)
 }
 EXPORT_SYMBOL(drm_plane_cleanup);
 
+int drm_live_source_init(struct drm_device *dev, struct drm_live_source *src,
+			 const char *name, unsigned long possible_planes,
+			 const uint32_t *formats, uint32_t format_count,
+			 const struct drm_live_source_funcs *funcs)
+{
+	unsigned int i;
+	int ret;
+
+	/* Multi-planar live sources are not supported for now. */
+	for (i = 0; i < format_count; ++i) {
+		if (drm_format_num_planes(formats[i]) != 1) {
+			DRM_DEBUG_KMS("multiplanar live sources unsupported\n");
+			return -EINVAL;
+		}
+	}
+
+	drm_modeset_lock_all(dev);
+
+	ret = drm_mode_object_get(dev, &src->base, DRM_MODE_OBJECT_LIVE_SOURCE);
+	if (ret)
+		goto out;
+
+	src->dev = dev;
+	src->funcs = funcs;
+	if (name)
+		strlcpy(src->name, name, DRM_SOURCE_NAME_LEN);
+	src->possible_planes = possible_planes;
+
+	src->format_types = kmalloc_array(format_count,
+					  sizeof(*src->format_types),
+					  GFP_KERNEL);
+	if (!src->format_types) {
+		DRM_DEBUG_KMS("out of memory when allocating source foramts\n");
+		drm_mode_object_put(dev, &src->base);
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	memcpy(src->format_types, formats,
+	       format_count * sizeof(*src->format_types));
+	src->format_count = format_count;
+
+	list_add_tail(&src->head, &dev->mode_config.live_source_list);
+	dev->mode_config.num_live_source++;
+
+ out:
+	drm_modeset_unlock_all(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_live_source_init);
+
+void drm_live_source_cleanup(struct drm_live_source *src)
+{
+	struct drm_device *dev = src->dev;
+
+	drm_modeset_lock_all(dev);
+	drm_mode_object_put(dev, &src->base);
+	list_del(&src->head);
+	dev->mode_config.num_live_source--;
+	drm_modeset_unlock_all(dev);
+}
+EXPORT_SYMBOL(drm_live_source_cleanup);
+
 /**
  * drm_plane_index - find the index of a registered plane
  * @plane: plane to find index for
@@ -2616,6 +2680,99 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
 }
 
 /**
+ * drm_mode_getsource_res - get live source info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return a live source and set of IDs.
+ */
+int drm_mode_getsource_res(struct drm_device *dev, void *data,
+			   struct drm_file *file_priv)
+{
+	struct drm_mode_get_live_source_res *src_resp = data;
+	struct drm_mode_config *config;
+	struct drm_live_source *src;
+	uint32_t __user *src_ptr;
+	int copied = 0, ret = 0;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	drm_modeset_lock_all(dev);
+	config = &dev->mode_config;
+
+	/*
+	 * This ioctl is called twice, once to determine how much space is
+	 * needed, and the 2nd time to fill it.
+	 */
+	if (config->num_live_source &&
+	    (src_resp->count_sources >= config->num_live_source)) {
+		src_ptr = (uint32_t __user *)(unsigned long)src_resp->source_id_ptr;
+
+		list_for_each_entry(src, &config->live_source_list, head) {
+			if (put_user(src->base.id, src_ptr + copied)) {
+				ret = -EFAULT;
+				goto out;
+			}
+			copied++;
+		}
+	}
+	src_resp->count_sources = config->num_live_source;
+
+out:
+	drm_modeset_unlock_all(dev);
+	return ret;
+}
+
+/**
+ * drm_mode_getsource - get live source info
+ * @dev: DRM device
+ * @data: ioctl data
+ * @file_priv: DRM file info
+ *
+ * Return live source info, including formats supported, ...
+ */
+int drm_mode_getsource(struct drm_device *dev, void *data,
+		       struct drm_file *file_priv)
+{
+	struct drm_mode_get_live_source *src_resp = data;
+	struct drm_mode_object *obj;
+	struct drm_live_source *src;
+
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return -EINVAL;
+
+	obj = drm_mode_object_find(dev, src_resp->source_id,
+				   DRM_MODE_OBJECT_LIVE_SOURCE);
+	if (!obj)
+		return -ENOENT;
+	src = obj_to_live_source(obj);
+
+	src_resp->source_id = src->base.id;
+	strlcpy(src_resp->name, src->name, DRM_SOURCE_NAME_LEN);
+	src_resp->possible_planes = src->possible_planes;
+
+	/*
+	 * This ioctl is called twice, once to determine how much space is
+	 * needed, and the 2nd time to fill it.
+	 */
+	if (src->format_count &&
+	    (src_resp->count_format_types >= src->format_count)) {
+		uint32_t __user *format_ptr;
+
+		format_ptr = (uint32_t __user *)(unsigned long)src_resp->format_type_ptr;
+		if (copy_to_user(format_ptr, src->format_types,
+				 sizeof(uint32_t) * src->format_count)) {
+			return -EFAULT;
+		}
+	}
+	src_resp->count_format_types = src->format_count;
+
+	return 0;
+}
+
+/**
  * drm_mode_set_config_internal - helper to call ->set_config
  * @set: modeset config to set
  *
@@ -5435,6 +5592,7 @@ void drm_mode_config_init(struct drm_device *dev)
 	INIT_LIST_HEAD(&dev->mode_config.property_list);
 	INIT_LIST_HEAD(&dev->mode_config.property_blob_list);
 	INIT_LIST_HEAD(&dev->mode_config.plane_list);
+	INIT_LIST_HEAD(&dev->mode_config.live_source_list);
 	idr_init(&dev->mode_config.crtc_idr);
 	idr_init(&dev->mode_config.tile_idr);
 
@@ -5474,6 +5632,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 	struct drm_property *property, *pt;
 	struct drm_property_blob *blob, *bt;
 	struct drm_plane *plane, *plt;
+	struct drm_live_source *src, *psrc;
 
 	list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
 				 head) {
@@ -5513,6 +5672,11 @@ void drm_mode_config_cleanup(struct drm_device *dev)
 		plane->funcs->destroy(plane);
 	}
 
+	list_for_each_entry_safe(src, psrc, &dev->mode_config.live_source_list,
+				 head) {
+		src->funcs->destroy(src);
+	}
+
 	list_for_each_entry_safe(crtc, ct, &dev->mode_config.crtc_list, head) {
 		crtc->funcs->destroy(crtc);
 	}
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 266dcd6cdf3b..e3b01a19f9ac 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -614,10 +614,12 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_PRIME_FD_TO_HANDLE, drm_prime_fd_to_handle_ioctl, DRM_AUTH|DRM_UNLOCKED|DRM_RENDER_ALLOW),
 
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANERESOURCES, drm_mode_getplane_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETSOURCERESOURCES, drm_mode_getsource_res, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETCRTC, drm_mode_setcrtc, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPLANE, drm_mode_getplane, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETPLANE, drm_mode_setplane, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETSOURCE, drm_mode_getsource, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR, drm_mode_cursor_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETGAMMA, drm_mode_gamma_get_ioctl, DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_SETGAMMA, drm_mode_gamma_set_ioctl, DRM_MASTER|DRM_UNLOCKED),
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 2e80ad1aea84..4d111d8a1782 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -53,6 +53,7 @@ struct fence;
 #define DRM_MODE_OBJECT_FB 0xfbfbfbfb
 #define DRM_MODE_OBJECT_BLOB 0xbbbbbbbb
 #define DRM_MODE_OBJECT_PLANE 0xeeeeeeee
+#define DRM_MODE_OBJECT_LIVE_SOURCE 0xe1e1e1e1
 #define DRM_MODE_OBJECT_ANY 0
 
 struct drm_mode_object {
@@ -246,6 +247,7 @@ struct drm_pending_vblank_event;
 struct drm_plane;
 struct drm_bridge;
 struct drm_atomic_state;
+struct drm_live_source;
 
 /**
  * struct drm_crtc_state - mutable CRTC state
@@ -868,6 +870,25 @@ struct drm_plane {
 	struct drm_plane_state *state;
 };
 
+struct drm_live_source_funcs {
+	void (*destroy)(struct drm_live_source *src);
+};
+
+struct drm_live_source {
+	struct drm_device *dev;
+	struct list_head head;
+
+	struct drm_mode_object base;
+
+	char name[DRM_SOURCE_NAME_LEN];
+
+	uint32_t possible_planes;
+	uint32_t *format_types;
+	uint32_t format_count;
+
+	const struct drm_live_source_funcs *funcs;
+};
+
 /**
  * struct drm_bridge_funcs - drm_bridge control functions
  * @attach: Called during drm_bridge_attach
@@ -1086,6 +1107,8 @@ struct drm_mode_config {
 	int num_overlay_plane;
 	int num_total_plane;
 	struct list_head plane_list;
+	int num_live_source;
+	struct list_head live_source_list;
 
 	int num_crtc;
 	struct list_head crtc_list;
@@ -1185,6 +1208,7 @@ struct drm_mode_config {
 #define obj_to_property(x) container_of(x, struct drm_property, base)
 #define obj_to_blob(x) container_of(x, struct drm_property_blob, base)
 #define obj_to_plane(x) container_of(x, struct drm_plane, base)
+#define obj_to_live_source(x) container_of(x, struct drm_live_source, base)
 
 struct drm_prop_enum_list {
 	int type;
@@ -1275,6 +1299,13 @@ extern int drm_crtc_check_viewport(const struct drm_crtc *crtc,
 
 extern void drm_encoder_cleanup(struct drm_encoder *encoder);
 
+extern int drm_live_source_init(struct drm_device *dev,
+				struct drm_live_source *src, const char *name,
+				unsigned long possible_planes,
+				const uint32_t *formats, uint32_t format_count,
+				const struct drm_live_source_funcs *funcs);
+extern void drm_live_source_cleanup(struct drm_live_source *src);
+
 extern const char *drm_get_connector_status_name(enum drm_connector_status status);
 extern const char *drm_get_subpixel_order_name(enum subpixel_order order);
 extern const char *drm_get_dpms_name(int val);
@@ -1390,6 +1421,8 @@ extern int drm_mode_getresources(struct drm_device *dev,
 				 void *data, struct drm_file *file_priv);
 extern int drm_mode_getplane_res(struct drm_device *dev, void *data,
 				   struct drm_file *file_priv);
+extern int drm_mode_getsource_res(struct drm_device *dev, void *data,
+				  struct drm_file *file_priv);
 extern int drm_mode_getcrtc(struct drm_device *dev,
 			    void *data, struct drm_file *file_priv);
 extern int drm_mode_getconnector(struct drm_device *dev,
@@ -1401,6 +1434,8 @@ extern int drm_mode_getplane(struct drm_device *dev,
 			       void *data, struct drm_file *file_priv);
 extern int drm_mode_setplane(struct drm_device *dev,
 			       void *data, struct drm_file *file_priv);
+extern int drm_mode_getsource(struct drm_device *dev,
+			      void *data, struct drm_file *file_priv);
 extern int drm_mode_cursor_ioctl(struct drm_device *dev,
 				void *data, struct drm_file *file_priv);
 extern int drm_mode_cursor2_ioctl(struct drm_device *dev,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index ff6ef62d084b..92621bd5194b 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -787,6 +787,9 @@ struct drm_prime_handle {
 #define DRM_IOCTL_MODE_CURSOR2		DRM_IOWR(0xBB, struct drm_mode_cursor2)
 #define DRM_IOCTL_MODE_ATOMIC		DRM_IOWR(0xBC, struct drm_mode_atomic)
 
+#define DRM_IOCTL_MODE_GETSOURCERESOURCES DRM_IOWR(0xBD, struct drm_mode_get_live_source_res)
+#define DRM_IOCTL_MODE_GETSOURCE	DRM_IOWR(0xBE, struct drm_mode_get_live_source)
+
 /**
  * Device specific ioctls should only be in their respective headers
  * The device specific ioctl range is from 0x40 to 0x9f.
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index dbeba949462a..e4d09f6f20eb 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -33,6 +33,7 @@
 #define DRM_CONNECTOR_NAME_LEN	32
 #define DRM_DISPLAY_MODE_LEN	32
 #define DRM_PROP_NAME_LEN	32
+#define DRM_SOURCE_NAME_LEN	32
 
 #define DRM_MODE_TYPE_BUILTIN	(1<<0)
 #define DRM_MODE_TYPE_CLOCK_C	((1<<1) | DRM_MODE_TYPE_BUILTIN)
@@ -179,6 +180,21 @@ struct drm_mode_get_plane_res {
 	__u32 count_planes;
 };
 
+struct drm_mode_get_live_source {
+	__u32 source_id;
+	char name[DRM_SOURCE_NAME_LEN];
+
+	__u32 possible_planes;
+
+	__u32 count_format_types;
+	__u64 format_type_ptr;
+};
+
+struct drm_mode_get_live_source_res {
+	__u64 source_id_ptr;
+	__u32 count_sources;
+};
+
 #define DRM_MODE_ENCODER_NONE	0
 #define DRM_MODE_ENCODER_DAC	1
 #define DRM_MODE_ENCODER_TMDS	2
-- 
2.0.5

^ permalink raw reply related

* [RFC/PATCH v2 2/5] drm: Connect live source to framebuffers
From: Laurent Pinchart @ 2015-04-13 18:39 UTC (permalink / raw)
  To: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Magnus Damm, Daniel Vetter
In-Reply-To: <1428950387-6913-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

Introduce a new live source flag for framebuffers. When a framebuffer is
created with that flag set, a live source is associated with the
framebuffer instead of buffer objects. The framebuffer can then be used
with a plane to connect it with the live source.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
 drivers/gpu/drm/drm_crtc.c  | 123 +++++++++++++++++++++++++++++++++++---------
 include/uapi/drm/drm_mode.h |   7 +++
 2 files changed, 107 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 1f71978b4f17..838fd5051a00 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3408,31 +3408,13 @@ static int format_check(const struct drm_mode_fb_cmd2 *r)
 	}
 }
 
-static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
+static int framebuffer_check_buffers(const struct drm_mode_fb_cmd2 *r,
+				     int hsub, int vsub)
 {
-	int ret, hsub, vsub, num_planes, i;
+	int num_planes, i;
 
-	ret = format_check(r);
-	if (ret) {
-		DRM_DEBUG_KMS("bad framebuffer format %s\n",
-			      drm_get_format_name(r->pixel_format));
-		return ret;
-	}
-
-	hsub = drm_format_horz_chroma_subsampling(r->pixel_format);
-	vsub = drm_format_vert_chroma_subsampling(r->pixel_format);
 	num_planes = drm_format_num_planes(r->pixel_format);
 
-	if (r->width == 0 || r->width % hsub) {
-		DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
-		return -EINVAL;
-	}
-
-	if (r->height == 0 || r->height % vsub) {
-		DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);
-		return -EINVAL;
-	}
-
 	for (i = 0; i < num_planes; i++) {
 		unsigned int width = r->width / (i != 0 ? hsub : 1);
 		unsigned int height = r->height / (i != 0 ? vsub : 1);
@@ -3464,6 +3446,100 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
 	return 0;
 }
 
+static int framebuffer_check_sources(struct drm_device *dev,
+				     const struct drm_mode_fb_cmd2 *r)
+{
+	struct drm_mode_object *obj;
+	struct drm_live_source *src;
+	unsigned int cpp;
+	unsigned int i;
+
+	/*
+	 * Ensure that userspace has zeroed unused handles, pitches, offsets and
+	 * modifiers to allow future API extensions.
+	 */
+	if (r->offsets[0] || r->modifier[0])
+		return -EINVAL;
+
+	for (i = 1; i < ARRAY_SIZE(r->handles); ++i) {
+		if (r->handles[i] || r->pitches[i] ||
+		    r->offsets[i] || r->modifier[i])
+			return -EINVAL;
+	}
+
+	/* Validate width, height and pitch. */
+	cpp = drm_format_plane_cpp(r->pixel_format, 0);
+
+	if ((uint64_t) r->width * cpp > UINT_MAX)
+		return -ERANGE;
+
+	if ((uint64_t) r->height * r->pitches[0] > UINT_MAX)
+		return -ERANGE;
+
+	if (r->pitches[0] != r->width * cpp) {
+		DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[0], i);
+		return -EINVAL;
+	}
+
+	/*
+	 * Find the live source and check whether it supports the requested
+	 * pixel format.
+	 */
+
+	obj = drm_mode_object_find(dev, r->handles[0],
+				   DRM_MODE_OBJECT_LIVE_SOURCE);
+	if (!obj) {
+		DRM_DEBUG_KMS("bad framebuffer source ID %u\n", r->handles[0]);
+		return -EINVAL;
+	}
+
+	src = obj_to_live_source(obj);
+
+	for (i = 0; i < src->format_count; i++) {
+		if (r->pixel_format == src->format_types[i])
+			break;
+	}
+
+	if (i == src->format_count) {
+		DRM_DEBUG_KMS("bad framebuffer pixel format 0x%08x for source %u\n",
+			      r->pixel_format, r->handles[0]);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int framebuffer_check(struct drm_device *dev,
+			     const struct drm_mode_fb_cmd2 *r)
+{
+	int ret, hsub, vsub;
+
+	ret = format_check(r);
+	if (ret) {
+		DRM_DEBUG_KMS("bad framebuffer format %s\n",
+			      drm_get_format_name(r->pixel_format));
+		return ret;
+	}
+
+	hsub = drm_format_horz_chroma_subsampling(r->pixel_format);
+	vsub = drm_format_vert_chroma_subsampling(r->pixel_format);
+
+	if (r->width == 0 || r->width % hsub) {
+		DRM_DEBUG_KMS("bad framebuffer width %u\n", r->width);
+		return -EINVAL;
+	}
+
+	if (r->height == 0 || r->height % vsub) {
+		DRM_DEBUG_KMS("bad framebuffer height %u\n", r->height);
+		return -EINVAL;
+	}
+
+	if (r->flags & DRM_MODE_FB_LIVE_SOURCE)
+		return framebuffer_check_sources(dev, r);
+	else
+		return framebuffer_check_buffers(r, hsub, vsub);
+}
+
 static struct drm_framebuffer *
 internal_framebuffer_create(struct drm_device *dev,
 			    struct drm_mode_fb_cmd2 *r,
@@ -3473,7 +3549,8 @@ internal_framebuffer_create(struct drm_device *dev,
 	struct drm_framebuffer *fb;
 	int ret;
 
-	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
+	if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS |
+			 DRM_MODE_FB_LIVE_SOURCE)) {
 		DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
 		return ERR_PTR(-EINVAL);
 	}
@@ -3495,7 +3572,7 @@ internal_framebuffer_create(struct drm_device *dev,
 		return ERR_PTR(-EINVAL);
 	}
 
-	ret = framebuffer_check(r);
+	ret = framebuffer_check(dev, r);
 	if (ret)
 		return ERR_PTR(ret);
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index e4d09f6f20eb..0cb73fb09d64 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -353,6 +353,7 @@ struct drm_mode_fb_cmd {
 
 #define DRM_MODE_FB_INTERLACED	(1<<0) /* for interlaced framebuffers */
 #define DRM_MODE_FB_MODIFIERS	(1<<1) /* enables ->modifer[] */
+#define DRM_MODE_FB_LIVE_SOURCE	(1<<2) /* connected to a live source */
 
 struct drm_mode_fb_cmd2 {
 	__u32 fb_id;
@@ -380,6 +381,12 @@ struct drm_mode_fb_cmd2 {
 	 * Vendor specific modifier token.  This allows, for example,
 	 * different tiling/swizzling pattern on different planes.
 	 * See discussion above of DRM_FORMAT_MOD_xxx.
+	 *
+	 * If the DRM_MODE_FB_LIVE_SOURCE flag is set the frame buffer input
+	 * comes from a live source instead of from memory. The handles[0]
+	 * field contains the ID of the connected live source object. All other
+	 * handles and all pitches, offsets and modifiers are then ignored by
+	 * the kernel and must be set to zero by applications.
 	 */
 	__u32 handles[4];
 	__u32 pitches[4]; /* pitch for each plane */
-- 
2.0.5

^ permalink raw reply related

* [RFC/PATCH v2 3/5] drm/rcar-du: Add VSP1 support to the planes allocator
From: Laurent Pinchart @ 2015-04-13 18:39 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, linux-api, linux-sh, Magnus Damm, linux-media
In-Reply-To: <1428950387-6913-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

The R8A7790 DU can source frames directly from the VSP1 devices VSPD0
and VSPD1. VSPD0 feeds DU0/1 plane 0, and VSPD1 feeds either DU2 plane 0
or DU0/1 plane 1.

Allocate the correct fixed plane when sourcing frames from VSPD0 or
VSPD1, and allocate planes in reverse index order otherwise to ensure
maximum availability of planes 0 and 1.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 40 ++++++++++++++++++++++++++++-----
 drivers/gpu/drm/rcar-du/rcar_du_plane.c |  1 +
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |  7 ++++++
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index fb052bca574f..17f89bfca8f8 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -244,11 +244,41 @@ static unsigned int rcar_du_plane_hwmask(struct rcar_du_plane_state *state)
 	return mask;
 }
 
-static int rcar_du_plane_hwalloc(unsigned int num_planes, unsigned int free)
+/*
+ * The R8A7790 DU can source frames directly from the VSP1 devices VSPD0 and
+ * VSPD1. VSPD0 feeds DU0/1 plane 0, and VSPD1 feeds either DU2 plane 0 or
+ * DU0/1 plane 1.
+ *
+ * Allocate the correct fixed plane when sourcing frames from VSPD0 or VSPD1,
+ * and allocate planes in reverse index order otherwise to ensure maximum
+ * availability of planes 0 and 1.
+ *
+ * The caller is responsible for ensuring that the requested source is
+ * compatible with the DU revision.
+ */
+static int rcar_du_plane_hwalloc(struct rcar_du_plane *plane,
+				 struct rcar_du_plane_state *state,
+				 unsigned int free)
 {
-	unsigned int i;
+	unsigned int num_planes = state->format->planes;
+	int fixed = -1;
+	int i;
+
+	if (state->source == RCAR_DU_PLANE_VSPD0) {
+		/* VSPD0 feeds plane 0 on DU0/1. */
+		if (plane->group->index != 0)
+			return -EINVAL;
+
+		fixed = 0;
+	} else if (state->source == RCAR_DU_PLANE_VSPD1) {
+		/* VSPD1 feeds plane 1 on DU0/1 or plane 0 on DU2. */
+		fixed = plane->group->index == 0 ? 1 : 0;
+	}
+
+	if (fixed >= 0)
+		return free & (1 << fixed) ? fixed : -EBUSY;
 
-	for (i = 0; i < RCAR_DU_NUM_HW_PLANES; ++i) {
+	for (i = RCAR_DU_NUM_HW_PLANES - 1; i >= 0; --i) {
 		if (!(free & (1 << i)))
 			continue;
 
@@ -256,7 +286,7 @@ static int rcar_du_plane_hwalloc(unsigned int num_planes, unsigned int free)
 			break;
 	}
 
-	return i == RCAR_DU_NUM_HW_PLANES ? -EBUSY : i;
+	return i < 0 ? -EBUSY : i;
 }
 
 static int rcar_du_atomic_check(struct drm_device *dev,
@@ -372,7 +402,7 @@ static int rcar_du_atomic_check(struct drm_device *dev,
 		    !rcar_du_plane_needs_realloc(plane, plane_state))
 			continue;
 
-		idx = rcar_du_plane_hwalloc(plane_state->format->planes,
+		idx = rcar_du_plane_hwalloc(plane, plane_state,
 					group_free_planes[plane->group->index]);
 		if (idx < 0) {
 			dev_dbg(rcdu->dev, "%s: no available hardware plane\n",
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 210e5c3fd982..80e4dba78aef 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -288,6 +288,7 @@ static void rcar_du_plane_reset(struct drm_plane *plane)
 		return;
 
 	state->hwindex = -1;
+	state->source = RCAR_DU_PLANE_MEMORY;
 	state->alpha = 255;
 	state->colorkey = RCAR_DU_COLORKEY_NONE;
 	state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
index abff0ebeb195..81c2d361a94f 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
@@ -28,6 +28,12 @@ struct rcar_du_group;
 #define RCAR_DU_NUM_KMS_PLANES		9
 #define RCAR_DU_NUM_HW_PLANES		8
 
+enum rcar_du_plane_source {
+	RCAR_DU_PLANE_MEMORY,
+	RCAR_DU_PLANE_VSPD0,
+	RCAR_DU_PLANE_VSPD1,
+};
+
 struct rcar_du_plane {
 	struct drm_plane plane;
 	struct rcar_du_group *group;
@@ -51,6 +57,7 @@ struct rcar_du_plane_state {
 
 	const struct rcar_du_format_info *format;
 	int hwindex;		/* 0-based, -1 means unused */
+	enum rcar_du_plane_source source;
 
 	unsigned int alpha;
 	unsigned int colorkey;
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [RFC/PATCH v2 4/5] drm/rcar-du: Add VSP1 live source support
From: Laurent Pinchart @ 2015-04-13 18:39 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, linux-api, linux-sh, Magnus Damm, linux-media
In-Reply-To: <1428950387-6913-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

Register live sources for VSPD0 and VSPD1 and configure the plane source
at plane setup time to source frames from memory or from the VSP1.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c   |   6 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.h   |   3 +
 drivers/gpu/drm/rcar-du/rcar_du_group.c |  19 ++--
 drivers/gpu/drm/rcar-du/rcar_du_group.h |   2 +
 drivers/gpu/drm/rcar-du/rcar_du_kms.c   |  92 ++++++++++++++++-
 drivers/gpu/drm/rcar-du/rcar_du_kms.h   |   3 +
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 168 +++++++++++++++++++++++++-------
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |   3 +
 drivers/gpu/drm/rcar-du/rcar_du_regs.h  |   1 +
 9 files changed, 246 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index da1216a73969..e16a912327b7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -58,7 +58,8 @@ static const struct rcar_du_device_info rcar_du_r8a7779_info = {
 
 static const struct rcar_du_device_info rcar_du_r8a7790_info = {
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
 	.quirks = RCAR_DU_QUIRK_ALIGN_128B | RCAR_DU_QUIRK_LVDS_LANES,
 	.num_crtcs = 3,
 	.routes = {
@@ -86,7 +87,8 @@ static const struct rcar_du_device_info rcar_du_r8a7790_info = {
 
 static const struct rcar_du_device_info rcar_du_r8a7791_info = {
 	.features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
-		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
+		  | RCAR_DU_FEATURE_EXT_CTRL_REGS
+		  | RCAR_DU_FEATURE_VSP1_SOURCE,
 	.num_crtcs = 2,
 	.routes = {
 		/* R8A7791 has one RGB output, one LVDS output and one
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index c7c538dd2e68..b5be16053e71 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -29,6 +29,7 @@ struct rcar_du_lvdsenc;
 
 #define RCAR_DU_FEATURE_CRTC_IRQ_CLOCK	(1 << 0)	/* Per-CRTC IRQ and clock */
 #define RCAR_DU_FEATURE_EXT_CTRL_REGS	(1 << 1)	/* Has extended control registers */
+#define RCAR_DU_FEATURE_VSP1_SOURCE	(1 << 2)	/* Has inputs from VSP1 */
 
 #define RCAR_DU_QUIRK_ALIGN_128B	(1 << 0)	/* Align pitches to 128 bytes */
 #define RCAR_DU_QUIRK_LVDS_LANES	(1 << 1)	/* LVDS lanes 1 and 3 inverted */
@@ -84,6 +85,8 @@ struct rcar_du_device {
 	struct rcar_du_group groups[RCAR_DU_MAX_GROUPS];
 
 	unsigned int dpad0_source;
+	unsigned int vspd1_sink;
+
 	struct rcar_du_lvdsenc *lvds[RCAR_DU_MAX_LVDS];
 
 	struct {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 1bdc0ee0c248..71f50bf45581 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -49,10 +49,13 @@ static void rcar_du_group_setup_defr8(struct rcar_du_group *rgrp)
 	u32 defr8 = DEFR8_CODE | DEFR8_DEFE8;
 
 	/* The DEFR8 register for the first group also controls RGB output
-	 * routing to DPAD0
+	 * routing to DPAD0 and VSPD1 routing to DU0/1/2.
 	 */
-	if (rgrp->index == 0)
+	if (rgrp->index == 0) {
 		defr8 |= DEFR8_DRGBS_DU(rgrp->dev->dpad0_source);
+		if (rgrp->dev->vspd1_sink == 2)
+			defr8 |= DEFR8_VSCS;
+	}
 
 	rcar_du_group_write(rgrp, DEFR8, defr8);
 }
@@ -155,17 +158,17 @@ void rcar_du_group_restart(struct rcar_du_group *rgrp)
 	__rcar_du_group_start_stop(rgrp, true);
 }
 
-static int rcar_du_set_dpad0_routing(struct rcar_du_device *rcdu)
+int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu)
 {
 	int ret;
 
 	if (!rcar_du_has(rcdu, RCAR_DU_FEATURE_EXT_CTRL_REGS))
 		return 0;
 
-	/* RGB output routing to DPAD0 is configured in the DEFR8 register of
-	 * the first group. As this function can be called with the DU0 and DU1
-	 * CRTCs disabled, we need to enable the first group clock before
-	 * accessing the register.
+	/* RGB output routing to DPAD0 and VSP1D routing to DU0/1/2 are
+	 * configured in the DEFR8 register of the first group. As this function
+	 * can be called with the DU0 and DU1 CRTCs disabled, we need to enable
+	 * the first group clock before accessing the register.
 	 */
 	ret = clk_prepare_enable(rcdu->crtcs[0].clock);
 	if (ret < 0)
@@ -196,5 +199,5 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 
 	rcar_du_group_write(rgrp, DORCR, dorcr);
 
-	return rcar_du_set_dpad0_routing(rgrp->dev);
+	return rcar_du_set_dpad0_vsp1_routing(rgrp->dev);
 }
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h b/drivers/gpu/drm/rcar-du/rcar_du_group.h
index ed36433fbe84..3fdf77171034 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h
@@ -52,4 +52,6 @@ void rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start);
 void rcar_du_group_restart(struct rcar_du_group *rgrp);
 int rcar_du_group_set_routing(struct rcar_du_group *rgrp);
 
+int rcar_du_set_dpad0_vsp1_routing(struct rcar_du_device *rcdu);
+
 #endif /* __RCAR_DU_GROUP_H__ */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 17f89bfca8f8..da22392cf379 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -134,6 +134,73 @@ int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
 	return drm_gem_cma_dumb_create_internal(file, dev, args);
 }
 
+struct rcar_du_live_framebuffer {
+	struct drm_framebuffer fb;
+	struct drm_live_source *src;
+};
+
+#define to_live_fb(fb) container_of(fb, struct rcar_du_live_framebuffer, fb)
+
+struct drm_live_source *rcar_du_live_fb_source(struct drm_framebuffer *fb)
+{
+	return fb->flags & DRM_MODE_FB_LIVE_SOURCE ? to_live_fb(fb)->src : NULL;
+}
+
+static void rcar_du_live_fb_destroy(struct drm_framebuffer *fb)
+{
+	struct rcar_du_live_framebuffer *live_fb = to_live_fb(fb);
+
+	drm_framebuffer_cleanup(fb);
+	kfree(live_fb);
+}
+
+static int rcar_du_live_fb_create_handle(struct drm_framebuffer *fb,
+	struct drm_file *file_priv, unsigned int *handle)
+{
+	*handle = 0;
+	return 0;
+}
+
+static struct drm_framebuffer_funcs rcar_du_live_fb_funcs = {
+	.destroy	= rcar_du_live_fb_destroy,
+	.create_handle	= rcar_du_live_fb_create_handle,
+};
+
+static struct drm_framebuffer *
+rcar_du_live_fb_create(struct drm_device *dev, struct drm_file *file_priv,
+		       struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	struct rcar_du_live_framebuffer *fb;
+	struct drm_mode_object *obj;
+	struct drm_live_source *src;
+	int ret;
+
+	obj = drm_mode_object_find(dev, mode_cmd->handles[0],
+				   DRM_MODE_OBJECT_LIVE_SOURCE);
+	if (!obj)
+		return ERR_PTR(-EINVAL);
+
+	src = obj_to_live_source(obj);
+
+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+	if (!fb)
+		return ERR_PTR(-ENOMEM);
+
+	drm_helper_mode_fill_fb_struct(&fb->fb, mode_cmd);
+
+	fb->src = src;
+
+	ret = drm_framebuffer_init(dev, &fb->fb, &rcar_du_live_fb_funcs);
+	if (ret) {
+		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n",
+			ret);
+		kfree(fb);
+		return ERR_PTR(ret);
+	}
+
+	return &fb->fb;
+}
+
 static struct drm_framebuffer *
 rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 		  struct drm_mode_fb_cmd2 *mode_cmd)
@@ -144,6 +211,9 @@ rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 	unsigned int align;
 	unsigned int bpp;
 
+	if (mode_cmd->flags & DRM_MODE_FB_LIVE_SOURCE)
+		return rcar_du_live_fb_create(dev, file_priv, mode_cmd);
+
 	format = rcar_du_format_info(mode_cmd->pixel_format);
 	if (format == NULL) {
 		dev_dbg(dev->dev, "unsupported pixel format %08x\n",
@@ -217,17 +287,25 @@ static void rcar_du_output_poll_changed(struct drm_device *dev)
  */
 
 static bool rcar_du_plane_needs_realloc(struct rcar_du_plane *plane,
-					struct rcar_du_plane_state *state)
+					struct rcar_du_plane_state *new_state)
 {
-	const struct rcar_du_format_info *cur_format;
+	struct rcar_du_plane_state *cur_state;
 
-	cur_format = to_rcar_du_plane_state(plane->plane.state)->format;
+	cur_state = to_rcar_du_plane_state(plane->plane.state);
 
 	/* Lowering the number of planes doesn't strictly require reallocation
 	 * as the extra hardware plane will be freed when committing, but doing
 	 * so could lead to more fragmentation.
 	 */
-	return !cur_format || cur_format->planes != state->format->planes;
+	if (!cur_state->format ||
+	    cur_state->format->planes != new_state->format->planes)
+		return true;
+
+	/* Reallocate hardware planes if the source has changed. */
+	if (cur_state->source != new_state->source)
+		return true;
+
+	return false;
 }
 
 static unsigned int rcar_du_plane_hwmask(struct rcar_du_plane_state *state)
@@ -776,6 +854,12 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 
 	drm_mode_config_reset(dev);
 
+	if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE)) {
+		ret = rcar_du_vsp1_sources_init(rcdu);
+		if (ret < 0)
+			return ret;
+	}
+
 	drm_kms_helper_poll_init(dev);
 
 	if (dev->mode_config.num_connector) {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.h b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
index 07951d5fe38b..b3c736f46231 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.h
@@ -18,6 +18,7 @@
 
 struct drm_file;
 struct drm_device;
+struct drm_live_source;
 struct drm_mode_create_dumb;
 struct rcar_du_device;
 
@@ -36,4 +37,6 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu);
 int rcar_du_dumb_create(struct drm_file *file, struct drm_device *dev,
 			struct drm_mode_create_dumb *args);
 
+struct drm_live_source *rcar_du_live_fb_source(struct drm_framebuffer *fb);
+
 #endif /* __RCAR_DU_KMS_H__ */
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 80e4dba78aef..07802639ac99 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -20,21 +20,84 @@
 #include <drm/drm_plane_helper.h>
 
 #include "rcar_du_drv.h"
+#include "rcar_du_group.h"
 #include "rcar_du_kms.h"
 #include "rcar_du_plane.h"
 #include "rcar_du_regs.h"
 
-#define RCAR_DU_COLORKEY_NONE		(0 << 24)
-#define RCAR_DU_COLORKEY_SOURCE		(1 << 24)
-#define RCAR_DU_COLORKEY_MASK		(1 << 24)
+/* -----------------------------------------------------------------------------
+ * Live Sources
+ */
+
+struct rcar_du_vsp1_source {
+	struct drm_live_source base;
+
+	enum rcar_du_plane_source source;
+};
+
+static inline struct rcar_du_vsp1_source *
+to_rcar_vsp1_source(struct drm_live_source *src)
+{
+	return container_of(src, struct rcar_du_vsp1_source, base);
+}
+
+static const struct drm_live_source_funcs rcar_du_live_source_funcs = {
+	.destroy = drm_live_source_cleanup,
+};
+
+static const uint32_t source_formats[] = {
+	DRM_FORMAT_XRGB8888,
+};
 
-static u32 rcar_du_plane_read(struct rcar_du_group *rgrp,
-			      unsigned int index, u32 reg)
+int rcar_du_vsp1_sources_init(struct rcar_du_device *rcdu)
 {
-	return rcar_du_read(rgrp->dev,
-			    rgrp->mmio_offset + index * PLANE_OFF + reg);
+	static const struct {
+		enum rcar_du_plane_source source;
+		unsigned int planes;
+	} sources[] = {
+		{ RCAR_DU_PLANE_VSPD0, BIT(RCAR_DU_NUM_KMS_PLANES - 1) },
+		{ RCAR_DU_PLANE_VSPD1, BIT(RCAR_DU_NUM_KMS_PLANES - 2) |
+				       BIT(2 * RCAR_DU_NUM_KMS_PLANES - 1) },
+	};
+	unsigned int planes_mask;
+	unsigned int num_planes;
+	unsigned int i;
+
+	num_planes = RCAR_DU_NUM_KMS_PLANES * DIV_ROUND_UP(rcdu->num_crtcs, 2);
+	planes_mask = (1 << num_planes) - 1;
+
+	for (i = 0; i < ARRAY_SIZE(sources); ++i) {
+		struct rcar_du_vsp1_source *src;
+		char name[6];
+		int ret;
+
+		src = devm_kzalloc(rcdu->dev, sizeof(*src), GFP_KERNEL);
+		if (src == NULL)
+			return -ENOMEM;
+
+		src->source = sources[i].source;
+
+		sprintf(name, "vspd%u", i);
+		ret = drm_live_source_init(rcdu->ddev, &src->base, name,
+					   sources[i].planes & planes_mask,
+					   source_formats,
+					   ARRAY_SIZE(source_formats),
+					   &rcar_du_live_source_funcs);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
 }
 
+/* -----------------------------------------------------------------------------
+ * Planes
+ */
+
+#define RCAR_DU_COLORKEY_NONE		(0 << 24)
+#define RCAR_DU_COLORKEY_SOURCE		(1 << 24)
+#define RCAR_DU_COLORKEY_MASK		(1 << 24)
+
 static void rcar_du_plane_write(struct rcar_du_group *rgrp,
 				unsigned int index, u32 reg, u32 data)
 {
@@ -42,34 +105,47 @@ static void rcar_du_plane_write(struct rcar_du_group *rgrp,
 		      data);
 }
 
-static void rcar_du_plane_setup_fb(struct rcar_du_plane *plane)
+static void rcar_du_plane_setup_scanout(struct rcar_du_plane *plane)
 {
 	struct rcar_du_plane_state *state =
 		to_rcar_du_plane_state(plane->plane.state);
-	struct drm_framebuffer *fb = plane->plane.state->fb;
 	struct rcar_du_group *rgrp = plane->group;
 	unsigned int src_x = state->state.src_x >> 16;
 	unsigned int src_y = state->state.src_y >> 16;
 	unsigned int index = state->hwindex;
-	struct drm_gem_cma_object *gem;
+	unsigned int pitch;
 	bool interlaced;
-	u32 mwr;
+	u32 dma[2];
 
 	interlaced = state->state.crtc->state->adjusted_mode.flags
 		   & DRM_MODE_FLAG_INTERLACE;
 
+	if (state->source == RCAR_DU_PLANE_MEMORY) {
+		struct drm_framebuffer *fb = state->state.fb;
+		struct drm_gem_cma_object *gem;
+		unsigned int i;
+
+		if (state->format->planes == 2)
+			pitch = fb->pitches[0];
+		else
+			pitch = fb->pitches[0] * 8 / state->format->bpp;
+
+		for (i = 0; i < state->format->planes; ++i) {
+			gem = drm_fb_cma_get_gem_obj(fb, i);
+			dma[i] = gem->paddr + fb->offsets[i];
+		}
+	} else {
+		pitch = state->state.src_w >> 16;
+		dma[0] = 0;
+		dma[1] = 0;
+	}
+
 	/* Memory pitch (expressed in pixels). Must be doubled for interlaced
 	 * operation with 32bpp formats.
 	 */
-	if (state->format->planes == 2)
-		mwr = fb->pitches[0];
-	else
-		mwr = fb->pitches[0] * 8 / state->format->bpp;
-
-	if (interlaced && state->format->bpp == 32)
-		mwr *= 2;
-
-	rcar_du_plane_write(rgrp, index, PnMWR, mwr);
+	rcar_du_plane_write(rgrp, index, PnMWR,
+			    (interlaced && state->format->bpp == 32) ?
+			    pitch * 2 : pitch);
 
 	/* The Y position is expressed in raster line units and must be doubled
 	 * for 32bpp formats, according to the R8A7790 datasheet. No mention of
@@ -87,21 +163,18 @@ static void rcar_du_plane_setup_fb(struct rcar_du_plane *plane)
 	rcar_du_plane_write(rgrp, index, PnSPYR, src_y *
 			    (!interlaced && state->format->bpp == 32 ? 2 : 1));
 
-	gem = drm_fb_cma_get_gem_obj(fb, 0);
-	rcar_du_plane_write(rgrp, index, PnDSA0R, gem->paddr + fb->offsets[0]);
+	rcar_du_plane_write(rgrp, index, PnDSA0R, dma[0]);
 
 	if (state->format->planes == 2) {
 		index = (index + 1) % 8;
 
-		rcar_du_plane_write(rgrp, index, PnMWR, fb->pitches[0]);
+		rcar_du_plane_write(rgrp, index, PnMWR, pitch);
 
 		rcar_du_plane_write(rgrp, index, PnSPXR, src_x);
 		rcar_du_plane_write(rgrp, index, PnSPYR, src_y *
 				    (state->format->bpp == 16 ? 2 : 1) / 2);
 
-		gem = drm_fb_cma_get_gem_obj(fb, 1);
-		rcar_du_plane_write(rgrp, index, PnDSA0R,
-				    gem->paddr + fb->offsets[1]);
+		rcar_du_plane_write(rgrp, index, PnDSA0R, dma[1]);
 	}
 }
 
@@ -168,8 +241,8 @@ static void rcar_du_plane_setup_mode(struct rcar_du_plane *plane,
 	}
 }
 
-static void __rcar_du_plane_setup(struct rcar_du_plane *plane,
-				  unsigned int index)
+static void rcar_du_plane_setup_format(struct rcar_du_plane *plane,
+				       unsigned int index)
 {
 	struct rcar_du_plane_state *state =
 		to_rcar_du_plane_state(plane->plane.state);
@@ -182,10 +255,6 @@ static void __rcar_du_plane_setup(struct rcar_du_plane *plane,
 	 * The data format is selected by the DDDF field in PnMR and the EDF
 	 * field in DDCR4.
 	 */
-	ddcr4 = rcar_du_plane_read(rgrp, index, PnDDCR4);
-	ddcr4 &= ~PnDDCR4_EDF_MASK;
-	ddcr4 |= state->format->edf | PnDDCR4_CODE;
-
 	rcar_du_plane_setup_mode(plane, index);
 
 	if (state->format->planes == 2) {
@@ -204,6 +273,11 @@ static void __rcar_du_plane_setup(struct rcar_du_plane *plane,
 	}
 
 	rcar_du_plane_write(rgrp, index, PnDDCR2, ddcr2);
+
+	ddcr4 = state->format->edf | PnDDCR4_CODE;
+	if (state->source != RCAR_DU_PLANE_MEMORY)
+		ddcr4 |= PnDDCR4_VSPS;
+
 	rcar_du_plane_write(rgrp, index, PnDDCR4, ddcr4);
 
 	/* Destination position and size */
@@ -224,11 +298,21 @@ void rcar_du_plane_setup(struct rcar_du_plane *plane)
 	struct rcar_du_plane_state *state =
 		to_rcar_du_plane_state(plane->plane.state);
 
-	__rcar_du_plane_setup(plane, state->hwindex);
+	rcar_du_plane_setup_format(plane, state->hwindex);
 	if (state->format->planes == 2)
-		__rcar_du_plane_setup(plane, (state->hwindex + 1) % 8);
+		rcar_du_plane_setup_format(plane, (state->hwindex + 1) % 8);
+
+	rcar_du_plane_setup_scanout(plane);
 
-	rcar_du_plane_setup_fb(plane);
+	if (state->source == RCAR_DU_PLANE_VSPD1) {
+		unsigned int vspd1_sink = plane->group->index ? 2 : 0;
+		struct rcar_du_device *rcdu = plane->group->dev;
+
+		if (rcdu->vspd1_sink != vspd1_sink) {
+			rcdu->vspd1_sink = vspd1_sink;
+			rcar_du_set_dpad0_vsp1_routing(rcdu);
+		}
+	}
 }
 
 static int rcar_du_plane_atomic_check(struct drm_plane *plane,
@@ -237,6 +321,7 @@ static int rcar_du_plane_atomic_check(struct drm_plane *plane,
 	struct rcar_du_plane_state *rstate = to_rcar_du_plane_state(state);
 	struct rcar_du_plane *rplane = to_rcar_plane(plane);
 	struct rcar_du_device *rcdu = rplane->group->dev;
+	uint32_t pixel_format;
 
 	if (!state->fb || !state->crtc) {
 		rstate->format = NULL;
@@ -249,13 +334,22 @@ static int rcar_du_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
-	rstate->format = rcar_du_format_info(state->fb->pixel_format);
+	pixel_format = state->fb->pixel_format;
+	rstate->format = rcar_du_format_info(pixel_format);
 	if (rstate->format == NULL) {
 		dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__,
-			state->fb->pixel_format);
+			pixel_format);
 		return -EINVAL;
 	}
 
+	if (state->fb->flags & DRM_MODE_FB_LIVE_SOURCE) {
+		struct drm_live_source *src = rcar_du_live_fb_source(state->fb);
+
+		rstate->source = to_rcar_vsp1_source(src)->source;
+	} else {
+		rstate->source = RCAR_DU_PLANE_MEMORY;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
index 81c2d361a94f..9a6132899d59 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
@@ -17,6 +17,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 
+struct rcar_du_device;
 struct rcar_du_format_info;
 struct rcar_du_group;
 
@@ -70,6 +71,8 @@ to_rcar_du_plane_state(struct drm_plane_state *state)
 	return container_of(state, struct rcar_du_plane_state, state);
 }
 
+int rcar_du_vsp1_sources_init(struct rcar_du_device *rcdu);
+
 int rcar_du_planes_init(struct rcar_du_group *rgrp);
 
 void rcar_du_plane_setup(struct rcar_du_plane *plane);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
index 70fcbc471ebd..ac9c3e511e79 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
@@ -389,6 +389,7 @@
 
 #define PnDDCR4			0x00190
 #define PnDDCR4_CODE		(0x7766 << 16)
+#define PnDDCR4_VSPS		(1 << 13)
 #define PnDDCR4_SDFS_RGB	(0 << 4)
 #define PnDDCR4_SDFS_YC		(5 << 4)
 #define PnDDCR4_SDFS_MASK	(7 << 4)
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [RFC/PATCH v2 5/5] drm/rcar-du: Restart the DU group when a plane source changes
From: Laurent Pinchart @ 2015-04-13 18:39 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, linux-api, linux-sh, Magnus Damm, linux-media
In-Reply-To: <1428950387-6913-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>

Plane sources are configured by the VSPS bit in the PnDDCR4 register.
Although the datasheet states that the bit is updated during vertical
blanking, it seems that updates only occur when the DU group is held in
reset through the DSYSR.DRES bit. Restart the group if the source
changes.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c  | 10 ++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_group.c |  2 ++
 drivers/gpu/drm/rcar-du/rcar_du_plane.c | 22 ++++++++++++++++++++--
 drivers/gpu/drm/rcar-du/rcar_du_plane.h |  1 +
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 7d0b8ef9bea2..969d49ab0d09 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -249,6 +249,8 @@ static void rcar_du_crtc_update_planes(struct rcar_du_crtc *rcrtc)
 		}
 	}
 
+	mutex_lock(&rcrtc->group->lock);
+
 	/* Select display timing and dot clock generator 2 for planes associated
 	 * with superposition controller 2.
 	 */
@@ -260,15 +262,19 @@ static void rcar_du_crtc_update_planes(struct rcar_du_crtc *rcrtc)
 		 * split, or through a module parameter). Flicker would then
 		 * occur only if we need to break the pre-association.
 		 */
-		mutex_lock(&rcrtc->group->lock);
 		if (rcar_du_group_read(rcrtc->group, DPTSR) != dptsr) {
 			rcar_du_group_write(rcrtc->group, DPTSR, dptsr);
 			if (rcrtc->group->used_crtcs)
 				rcar_du_group_restart(rcrtc->group);
 		}
-		mutex_unlock(&rcrtc->group->lock);
 	}
 
+	/* Restart the group if plane sources have changed. */
+	if (rcrtc->group->planes.need_restart)
+		rcar_du_group_restart(rcrtc->group);
+
+	mutex_unlock(&rcrtc->group->lock);
+
 	rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR,
 			    dspr);
 }
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c b/drivers/gpu/drm/rcar-du/rcar_du_group.c
index 71f50bf45581..101997e6e531 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
@@ -154,6 +154,8 @@ void rcar_du_group_start_stop(struct rcar_du_group *rgrp, bool start)
 
 void rcar_du_group_restart(struct rcar_du_group *rgrp)
 {
+	rgrp->planes.need_restart = false;
+
 	__rcar_du_group_start_stop(rgrp, false);
 	__rcar_du_group_start_stop(rgrp, true);
 }
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
index 07802639ac99..0bf2aaaf91e6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
@@ -357,9 +357,27 @@ static void rcar_du_plane_atomic_update(struct drm_plane *plane,
 					struct drm_plane_state *old_state)
 {
 	struct rcar_du_plane *rplane = to_rcar_plane(plane);
+	struct rcar_du_plane_state *old_rstate;
+	struct rcar_du_plane_state *new_rstate;
 
-	if (plane->state->crtc)
-		rcar_du_plane_setup(rplane);
+	if (!plane->state->crtc)
+		return;
+
+	rcar_du_plane_setup(rplane);
+
+	/* Check whether the source has changed from memory to live source or
+	 * from live source to memory. The source has been configured by the
+	 * VSPS bit in the PnDDCR4 register. Although the datasheet states that
+	 * the bit is updated during vertical blanking, it seems that updates
+	 * only occur when the DU group is held in reset through the DSYSR.DRES
+	 * bit. We thus need to restart the group if the source changes.
+	 */
+	old_rstate = to_rcar_du_plane_state(old_state);
+	new_rstate = to_rcar_du_plane_state(plane->state);
+
+	if ((old_rstate->source == RCAR_DU_PLANE_MEMORY) !=
+	    (new_rstate->source == RCAR_DU_PLANE_MEMORY))
+		rplane->group->planes.need_restart = true;
 }
 
 static const struct drm_plane_helper_funcs rcar_du_plane_helper_funcs = {
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
index 9a6132899d59..694b44c151b6 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h
@@ -47,6 +47,7 @@ static inline struct rcar_du_plane *to_rcar_plane(struct drm_plane *plane)
 
 struct rcar_du_planes {
 	struct rcar_du_plane planes[RCAR_DU_NUM_KMS_PLANES];
+	bool need_restart;
 
 	struct drm_property *alpha;
 	struct drm_property *colorkey;
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* Re: [patch 4/4] timerfd.2: Add ioctl method description
From: Michael Kerrisk (man-pages) @ 2015-04-13 19:56 UTC (permalink / raw)
  To: Cyrill Gorcunov, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Thomas Gleixner,
	Andrew Morton, Andrey Vagin, Pavel Emelyanov, Vladimir Davydov
In-Reply-To: <20140715215703.353177995-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>

Hello Cyrill,

On 07/15/2014 11:54 PM, Cyrill Gorcunov wrote:
> ioctl(2)
> 	The following commands are supported: TFD_IOC_SET_TICKS to adjust
> 	the number of the timer expirations that have occurred.
> 	It take a pointer to nonzero 8-byte integer (uint64_t*) containing
> 	new number of expirations. Once the number is set any waiter on
> 	the timer is woken up. The only purpose of this command is to restore
> 	the expirations in a sake of checkpoint/restore procedure.
> 	It requires the kernel to be built with CONFIG_CHECKPOINT_RESTORE
> 	support.

Late... but, patch applied. Thank you.

Cheers,

Michael


> Signed-off-by: Cyrill Gorcunov <gorcunov-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
> CC: Michael Kerrisk <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> CC: Andrey Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
> CC: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> CC: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  man2/timerfd_create.2 |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> Index: man-pages/man2/timerfd_create.2
> ===================================================================
> --- man-pages.orig/man2/timerfd_create.2
> +++ man-pages/man2/timerfd_create.2
> @@ -260,6 +260,20 @@ multiplexing APIs:
>  and
>  .BR epoll (7).
>  .TP
> +.BR ioctl "(2)"
> +The following commands are supported:
> +.B TFD_IOC_SET_TICKS
> +to adjust the number of the timer expirations that have occurred.
> +It take a pointer to nonzero 8-byte integer
> +.RI ( uint64_t *)
> +containing the new number of expirations.
> +Once the number is set any waiter on the timer is woken up.
> +The only purpose of this command is to restore the expirations
> +in a sake of checkpoint/restore procedure.
> +It requires the kernel to be built with
> +.BR CONFIG_CHECKPOINT_RESTORE
> +support.
> +.TP
>  .BR close (2)
>  When the file descriptor is no longer required it should be closed.
>  When all file descriptors associated with the same timer object
> 
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply

* Re: [patch 4/4] timerfd.2: Add ioctl method description
From: Cyrill Gorcunov @ 2015-04-13 20:10 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages)
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Thomas Gleixner, Andrew Morton,
	Andrey Vagin, Pavel Emelyanov, Vladimir Davydov
In-Reply-To: <552C1F64.30202-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Mon, Apr 13, 2015 at 09:56:20PM +0200, Michael Kerrisk (man-pages) wrote:
> Hello Cyrill,
> 
> On 07/15/2014 11:54 PM, Cyrill Gorcunov wrote:
> > ioctl(2)
> > 	The following commands are supported: TFD_IOC_SET_TICKS to adjust
> > 	the number of the timer expirations that have occurred.
> > 	It take a pointer to nonzero 8-byte integer (uint64_t*) containing
> > 	new number of expirations. Once the number is set any waiter on
> > 	the timer is woken up. The only purpose of this command is to restore
> > 	the expirations in a sake of checkpoint/restore procedure.
> > 	It requires the kernel to be built with CONFIG_CHECKPOINT_RESTORE
> > 	support.
> 
> Late... but, patch applied. Thank you.

Sure thing! Ping me if anything else should be added.

^ permalink raw reply

* Re: [v12 0/5] ext4: add project quota support
From: Jan Kara @ 2015-04-14  8:21 UTC (permalink / raw)
  To: Alban Crequy
  Cc: adilger-m1MBpc4rdrD3fQ9qLvQP4Q, tytso-3s7WtUTddSA, Linux API,
	Linux Containers, hch-wEGCiKHe2LqWVfeAwA7xHQ,
	dmonakhov-GEFAQzZX7r8dnm+yROfE0A,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, Li Xi,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, jack-AlSwsSmVLrQ,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAMXgnP6RF4HPDyugvnMKn3rDnuG7j1cz-xFtEMWx4Va1rEHVEQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sun 12-04-15 17:36:53, Alban Crequy wrote:
> 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.
  I don't think overlay's workdir needs project id. Limits will be simply
checked when storing data into upperdir by overlayfs. Overlayfs will get
EDQUOT which it will report back into the user.

> 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.
  So I don't know much about namespaces but I don't see how quota netlink
messages would be connected with *user* namespaces. But you are right that
quota netlink messages will contain ID of the violator mapped into init
user namespace so it won't make sense to processes in other user namespaces
even if they were able to receive it.

> 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?
  Quota netlink messages are sent only in init_net namespace (since quota
netlink protocol wasn't made namespace aware). So this shouldn't be an
issue.
 
> 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.
  Correct. This raises a somewhat unrelated question: Does this mean that a
container is able to mount arbitrary block device? Because also there we
just pass a device path to the kernel...

> 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.
  Yes.

> 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).
  Yes.

> 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
  I have added that.

> information about projid. But maybe this belong to the proc(5) and
> user_namespaces(7) manpages as well.
  Project ID in VFS quotas is fairly new thing. Once ext4 gains support for
it, I can add some documentation.

> Is there any suggestions how to allocate projid in userspace?
> Something like /etc/subprojid similar to /etc/subuid?
  I guess you need some coordination between namespaces? I only know that
traditionally xfsprogs use /etc/projid for name->project id translation 
and /etc/projects contain roots of directory trees for which you wish to
maintain directory quota together with project ids for each of the trees.

								Honza
-- 
Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH v2 02/10] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
From: Christoffer Dall @ 2015-04-14  8:24 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kvm, linux-arm-kernel, kvmarm, marc.zyngier, peter.maydell, agraf,
	drjones, pbonzini, zhichao.huang, jan.kiszka, dahi, r65777, bp,
	Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	maintainer:X86 ARCHITECTURE..., Gleb Natapov, Bharat Bhushan,
	Alexey Kardashevskiy, Mihai Caraman, Nadav Amit,
	open list:LINUX FOR POWERPC..., open list
In-Reply-To: <871tjokrnu.fsf@linaro.org>

On Mon, Apr 13, 2015 at 03:51:33PM +0100, Alex Bennée wrote:
> 
> Christoffer Dall <christoffer.dall@linaro.org> writes:
> 
> > 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,
> >
> > can you be more specific than 'stuff' here?  features?
> >
> >> + * however there is some shared commonality for the common cases
> >
> > I don't like this sentence; shared commonality is a pleonasm and the use
> > of however makes it sounds like there's some caveat here.
> 
> OK I can see that - after I looked it up ;-)
> 
> > If the top 16 bits are indeed arhictecture specific, then I think they
> > should just be defined in their architecture specific headers.  Unless
> > the idea here is that there's a fixed set of of flags that architectures
> > can choose to support, in which case it should simply be defined in the
> > common header.
> 
> Well an architecture might not support some features and want to use
> those bits for something else? I didn't want to force the bottom two
> of the architecture specific bits to wasted if the features don't exist.
> 
In that case I think the definition is local to each architecture and
should indeed just be duplicated.  The __ definitions complicate more
than they help as they are exported to userspace etc.  The KVM
maintainers may have a different view on this though.

-Christoffer

^ permalink raw reply

* Re: [RFC/PATCH v2 0/5] Add live source objects to DRM
From: Daniel Vetter @ 2015-04-14  9:56 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Daniel Vetter,
	linux-api-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA,
	Magnus Damm, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1428950387-6913-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

On Mon, Apr 13, 2015 at 09:39:42PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> Here's a proposal for a different approach to live source in DRM based on an
> idea by Daniel Vetter. The previous version can be found at
> http://lists.freedesktop.org/archives/dri-devel/2015-March/079319.html.

msm is also interested in a drm/v4l bridge on the capture/writeback side.
Although I haven't convinced them yet that rolling their own isn't
awesome.

> The need comes from the Renesas R-Car SoCs in which a video processing engine
> (named VSP1) that operates from memory to memory has one output directly
> connected to a plane of the display engine (DU) without going through memory.
> 
> The VSP1 is supported by a V4L2 driver. While it could be argued that it
> should instead be supported directly by the DRM rcar-du driver, this wouldn't
> be a good idea for at least two reasons. First, the R-Car SoCs contain several
> VSP1 instances, of which only a subset have a direct DU connection. The only
> other instances operate solely in memory to memory mode. Then, the VSP1 is a
> video processing engine and not a display engine. Its features are easily
> supported by the V4L2 API, but don't map to the DRM/KMS API. Significant
> changes to DRM/KMS would be required, beyond what is in my opinion an
> acceptable scope for a display API.
> 
> Now that the need to interface two separate devices supported by two different
> drivers in two separate subsystems has been established, we need an API to do
> so. It should be noted that while that API doesn't exist in the mainline
> kernel, the need isn't limited to Renesas SoCs.
> 
> This patch set proposes one possible solution for the problem in the form of a
> new DRM object named live source. Live sources are created by drivers to model
> hardware connections between a plane input and an external source, and are
> attached to planes through the KMS userspace API.
> 
> Patch 1/5 adds live source objects to DRM, with an in-kernel API for drivers
> to register the sources, and a userspace API to enumerate them.
> 
> Patch 2/5 implements connection between live sources and planes through
> framebuffers. It introduces a new live source flag for framebuffers. When a
> framebuffer is created with that flag set, a live source is associated with
> the framebuffer instead of buffer objects. The framebuffer can then be used
> with a plane to connect it with the live source. This is the biggest
> difference compared to the previous approach, and has several benefits:
> 
> - Changes are less intrusive in the DRM core
> - The implementation supports both the legacy API and atomic updates without
>   any code specific to either
> - No changes to existing drivers are needed
> - The framebuffer format and size configuration API is reused
> 
> The framebuffer format and size should ideally be validated using information
> queried directly from the driver that supports the live source device, but
> I've decided not to implement such communication between V4L2 and DRM/KMS at
> the moment to keep the proposal simple.
> 
> Patches 3/5 to 5/5 then implement support for live sources in the R-Car DU
> driver. The rcar_du_live_framebuffer structure and its associated helper
> functions could be moved to the DRM core later if other drivers need a similar
> implementation. I've decided to keep them in the rcar-du driver for now as
> it's not clear yet what other drivers might need.
> 
> Once again nothing here is set in stone.

Yeah, this looks rather nice&tidy. A few questions/ideas:
- Should we also go right ahead and add live sinks here with this and
  enumerate both live sinks and sources as live resources or something
  similar? The only big difference I see is that sinks will have different
  attachment points than sources.

- Not fully sure about possible_planes. I guess if the justification is
  that the drm core can take care of some input validation for drivers
  then it's useful. But if the idea is that userspace can use this to
  figure out the routing then I think this won't work.

- Do we need other basic checks like max/min width/height? Same concern as
  above.

- I think a live_resource_create callback would be useful so that the
  driver can check additional constraints (maybe just some size will work
  if e.g. it feeds directly into an mpeg encoder). Also I think we should
  figure out whether the addfb2 call or only attaching the live
  source/sink locks down the configuration on the v4l side of things.
  Imo locking down the settings has the advantage that you can do that all
  upfront on both v4l and drm side, and then immediately start the
  pipeline with just connecting it.

- Some helper to connect the drm and v4l side of things might be useful,
  especially to make sure you don't end up with incompatible settings on
  either end. Ties in with the above question of where we want to lock
  down the settings.

- Do we need some additional properties on live sources/sinks to at least
  make the connection with v4l? Or is the idea that we'll do this with the
  media link framework (in which case another part of some helper)?

Ok that's it for first thoughts from me.
-Daniel

> 
> Laurent Pinchart (5):
>   drm: Add live source object
>   drm: Connect live source to framebuffers
>   drm/rcar-du: Add VSP1 support to the planes allocator
>   drm/rcar-du: Add VSP1 live source support
>   drm/rcar-du: Restart the DU group when a plane source changes
> 
>  drivers/gpu/drm/drm_crtc.c              | 287 +++++++++++++++++++++++++++++---
>  drivers/gpu/drm/drm_ioctl.c             |   2 +
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c  |  10 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c   |   6 +-
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h   |   3 +
>  drivers/gpu/drm/rcar-du/rcar_du_group.c |  21 ++-
>  drivers/gpu/drm/rcar-du/rcar_du_group.h |   2 +
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c   | 132 ++++++++++++++-
>  drivers/gpu/drm/rcar-du/rcar_du_kms.h   |   3 +
>  drivers/gpu/drm/rcar-du/rcar_du_plane.c | 191 ++++++++++++++++-----
>  drivers/gpu/drm/rcar-du/rcar_du_plane.h |  11 ++
>  drivers/gpu/drm/rcar-du/rcar_du_regs.h  |   1 +
>  include/drm/drm_crtc.h                  |  35 ++++
>  include/uapi/drm/drm.h                  |   3 +
>  include/uapi/drm/drm_mode.h             |  23 +++
>  15 files changed, 647 insertions(+), 83 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply

* [PATCH] virtio_balloon: drop virtio_balloon_stat_modern
From: Michael S. Tsirkin @ 2015-04-14 10:01 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-api, virtualization

Looks like we are better off sticking with the misaligned stat struct,
to reduce the amount of virtio 1 specific code in balloon.  So let's do
it.

Add a detailed comment to reduce the chance people copy this bad example.

This also fixes a bug on BE architectures: tag should use
cpu_to_le16, not cpu_to_le32.

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Just reposting so it's easier to apply.
Feel free to squash into previous patch if you think it's
neater.

 include/uapi/linux/virtio_balloon.h | 33 +++++++++++++++++++++++++--------
 drivers/virtio/virtio_balloon.c     | 19 ++++---------------
 2 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index f81b220..984169a 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -52,15 +52,32 @@ struct virtio_balloon_config {
 #define VIRTIO_BALLOON_S_MEMTOT   5   /* Total amount of memory */
 #define VIRTIO_BALLOON_S_NR       6
 
+/*
+ * Memory statistics structure.
+ * Driver fills an array of these structures and passes to device.
+ *
+ * NOTE: fields are laid out in a way that would make compiler add padding
+ * between and after fields, so we have to use compiler-specific attributes to
+ * pack it, to disable this padding. This also often causes compiler to
+ * generate suboptimal code.
+ *
+ * We maintain this statistics structure format for backwards compatibility,
+ * but don't follow this example.
+ *
+ * If implementing a similar structure, do something like the below instead:
+ *     struct virtio_balloon_stat {
+ *         __virtio16 tag;
+ *         __u8 reserved[6];
+ *         __virtio64 val;
+ *     };
+ *
+ * In other words, add explicit reserved fields to align field and
+ * structure boundaries at field size, avoiding compiler padding
+ * without the packed attribute.
+ */
 struct virtio_balloon_stat {
-	__u16 tag;
-	__u64 val;
+	__virtio16 tag;
+	__virtio64 val;
 } __attribute__((packed));
 
-struct virtio_balloon_stat_modern {
-	__le16 tag;
-	__u8 reserved[6];
-	__le64 val;
-};
-
 #endif /* _LINUX_VIRTIO_BALLOON_H */
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0583720..9db546e 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -77,10 +77,7 @@ struct virtio_balloon {
 
 	/* Memory statistics */
 	int need_stats_update;
-	union {
-		struct virtio_balloon_stat_modern stats[VIRTIO_BALLOON_S_NR];
-		struct virtio_balloon_stat legacy_stats[VIRTIO_BALLOON_S_NR];
-	};
+	struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
 
 	/* To register callback in oom notifier call chain */
 	struct notifier_block nb;
@@ -93,10 +90,7 @@ static struct virtio_device_id id_table[] = {
 
 static void stats_sg_init(struct virtio_balloon *vb, struct scatterlist *sg)
 {
-	if (virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1))
-		sg_init_one(sg, vb->stats, sizeof(vb->stats));
-	else
-		sg_init_one(sg, vb->legacy_stats, sizeof(vb->legacy_stats));
+	sg_init_one(sg, vb->stats, sizeof(vb->stats));
 }
 
 static u32 page_to_balloon_pfn(struct page *page)
@@ -225,13 +219,8 @@ static inline void update_stat(struct virtio_balloon *vb, int idx,
 			       u16 tag, u64 val)
 {
 	BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
-	if (virtio_has_feature(vb->vdev, VIRTIO_F_VERSION_1)) {
-		vb->stats[idx].tag = cpu_to_le32(tag);
-		vb->stats[idx].val = cpu_to_le64(val);
-	} else {
-		vb->legacy_stats[idx].tag = tag;
-		vb->legacy_stats[idx].val = val;
-	}
+	vb->stats[idx].tag = cpu_to_virtio16(vb->vdev, tag);
+	vb->stats[idx].val = cpu_to_virtio64(vb->vdev, val);
 }
 
 #define pages_to_bytes(x) ((u64)(x) << PAGE_SHIFT)
-- 
MST

^ permalink raw reply related

* Re: [v12 0/5] ext4: add project quota support
From: Alban Crequy @ 2015-04-14 10:07 UTC (permalink / raw)
  To: Jan Kara
  Cc: Alban Crequy, adilger-m1MBpc4rdrD3fQ9qLvQP4Q, tytso-3s7WtUTddSA,
	Linux API, Linux Containers, hch-wEGCiKHe2LqWVfeAwA7xHQ,
	dmonakhov-GEFAQzZX7r8dnm+yROfE0A,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, Li Xi,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-ext4-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150414082115.GB23327-+0h/O2h83AeN3ZZ/Hiejyg@public.gmane.org>

On Tue, Apr 14, 2015 at 10:21 AM, Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org> wrote:
> On Sun 12-04-15 17:36:53, Alban Crequy wrote:
>> 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.
>   I don't think overlay's workdir needs project id. Limits will be simply
> checked when storing data into upperdir by overlayfs. Overlayfs will get
> EDQUOT which it will report back into the user.

Noted, thanks.

>> 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.
>   So I don't know much about namespaces but I don't see how quota netlink
> messages would be connected with *user* namespaces. But you are right that
> quota netlink messages will contain ID of the violator mapped into init
> user namespace so it won't make sense to processes in other user namespaces
> even if they were able to receive it.
>
>> 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?
>   Quota netlink messages are sent only in init_net namespace (since quota
> netlink protocol wasn't made namespace aware). So this shouldn't be an
> issue.

You're right, I misread it, it references the init network namespace
and not the user namespace:

fs/quota/netlink.c:quota_send_warning() uses genlmsg_multicast() which
specifically references init_net:

         return genlmsg_multicast_netns(family, &init_net, skb,
                                        portid, group, flags);

>> 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.
>   Correct. This raises a somewhat unrelated question: Does this mean that a
> container is able to mount arbitrary block device? Because also there we
> just pass a device path to the kernel...

The process would still need CAP_SYS_ADMIN and there are additional
checks when the user namespace is not the initial user namespace:

fs/namespace.c do_new_mount()
        if (user_ns != &init_user_ns) {
                if (!(type->fs_flags & FS_USERNS_MOUNT)) {
                        put_filesystem(type);
                        return -EPERM;
                }...

For example, FS_USERNS_MOUNT is set on devpts_fs_type but not on
ext4_fs_type. So it's not possible to mount ext4 in a different user
namespace. Containers that don't use user namespaces can avoid giving
CAP_SYS_ADMIN or restrict mount with some AppArmor rules.

>> 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.
>   Yes.
>
>> 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).
>   Yes.
>
>> 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
>   I have added that.
>
>> information about projid. But maybe this belong to the proc(5) and
>> user_namespaces(7) manpages as well.
>   Project ID in VFS quotas is fairly new thing. Once ext4 gains support for
> it, I can add some documentation.
>
>> Is there any suggestions how to allocate projid in userspace?
>> Something like /etc/subprojid similar to /etc/subuid?
>   I guess you need some coordination between namespaces?

Yes, I was thinking if Docker uses projid for some containers, rkt
uses other projid for other containers and the sysadmin also define
some projid manually.

> I only know that
> traditionally xfsprogs use /etc/projid for name->project id translation
> and /etc/projects contain roots of directory trees for which you wish to
> maintain directory quota together with project ids for each of the trees.

Thanks for the pointer.

Alban

>
>                                                                 Honza
> --
> Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
> SUSE Labs, CR
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [PATCH] virtio_balloon: drop virtio_balloon_stat_modern
From: Cornelia Huck @ 2015-04-14 10:13 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rusty Russell,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1429005625-23181-1-git-send-email-mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Tue, 14 Apr 2015 12:01:13 +0200
"Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Looks like we are better off sticking with the misaligned stat struct,
> to reduce the amount of virtio 1 specific code in balloon.  So let's do
> it.
> 
> Add a detailed comment to reduce the chance people copy this bad example.
> 
> This also fixes a bug on BE architectures: tag should use
> cpu_to_le16, not cpu_to_le32.
> 
> Acked-by: Cornelia Huck <cornelia.huck-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 
> Just reposting so it's easier to apply.
> Feel free to squash into previous patch if you think it's
> neater.

+1 for squashing from me. My A-b would also apply to the squashed patch.
> 
>  include/uapi/linux/virtio_balloon.h | 33 +++++++++++++++++++++++++--------
>  drivers/virtio/virtio_balloon.c     | 19 ++++---------------
>  2 files changed, 29 insertions(+), 23 deletions(-)

^ permalink raw reply

* Re: [PATCH v2 08/10] KVM: arm64: guest debug, HW assisted debug support
From: Christoffer Dall @ 2015-04-14 10:17 UTC (permalink / raw)
  To: Alex Bennée
  Cc: kvm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg, marc.zyngier-5wv7dgnIgG8,
	peter.maydell-QSEj5FYQhm4dnm+yROfE0A, agraf-l3A5Bk7waGM,
	drjones-H+wXaHxf7aLQT0dZR+AlfA, pbonzini-H+wXaHxf7aLQT0dZR+AlfA,
	zhichao.huang-QSEj5FYQhm4dnm+yROfE0A,
	jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ,
	dahi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	r65777-KZfg59tc24xl57MIdRCFDg, bp-l3A5Bk7waGM, Gleb Natapov,
	Jonathan Corbet, Russell King, Catalin Marinas, Will Deacon,
	AKASHI Takahiro, Lorenzo Pieralisi, open list:DOCUMENTATION,
	open list, open list:ABI/API
In-Reply-To: <1427814488-28467-9-git-send-email-alex.bennee-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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 don't really understand what this sentence means?

> 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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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.
> +

can you document their behavior more specifically?  I assume they both
return 0 if HW assisted debugging is not supported and return the number
of implemented hardware registers otherwise?

How does this work on big.LITTLE systems where cores may have a different
number of implemented registers?

>  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.
> +			 */
> +			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);
> +
> +	/*
> +	 * If we are not treating debug registers are dirty we need
> +	 * to trap if the guest starts accessing them.
> +	 */

Hmmm, this comment could have been added when introducing the original
logic instead.  Also:

s/are/as/

I'm not sure the 'treating them as dirty' part of the comment is
helpful.  Perhaps you want something like:

  If the guest debug register state is dirty (the guest is actively
  accesing them), then we context-switch the registers in EL2.
  Otherwise, we trap-and-emulate all guest accesses to 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;
>  
>  	/* 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();

again, don't you have to be careful to be in a non-preemptible section
here?

> +			struct kvm_guest_debug_arch *saved, *host;
> +
> +			vcpu_sys_reg(vcpu, MDSCR_EL1) |=
> +				(DBG_MDSCR_KDE|DBG_MDSCR_MDE);

Can you always set the KDE bit?  What if you only want debug exceptions
from guest user mode?  Is that supported through the KVM interface
and/or GDB?

style: spaces around the '|' operator.

> +
> +			/*
> +			 * First we need to make a copy of the guest
> +			 * debug registers before we wipe them out
> +			 * with the ones we want to use.
> +			 */

ok, I may be over-doing this, but you've sort of gotten me on the track
here; First typically implies that there's a Second, or subsequently, or
something like that here.

Also, regarding 'need to' vs. 'must', I found this a nice read:
http://www.kirkmahoney.com/blog/2008/02/must-vs-needs-to/

I think this comment would be more clear if it was written something
like:

  We are about to set the hw debug registers with values used to debug
  the VM.  Make a copy of the guest debug register state to preserve
  the guest's view of the debug register state.

> +			saved = &vcpu_debug_saved_reg(vcpu, debug_regs);
> +			host = &vcpu->arch.guest_debug_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);

you need spaces around all the '*' operators.


Instead of copying back and forth, could we just have a pointer on the
vcpu->arch struct to the debug regs struct that the EL2 code will
context-switch (if trapping is disabled) and then just shift the pointer
in C code instead of doing all this?

> +
> +			/* 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);
> +
> +			/* Restore the saved debug register values */
> +			memcpy(&vcpu_sys_reg(vcpu, DBGBCR0_EL1),
> +			       &regs->dbg_bcr,
> +			       sizeof(__u64)*nb);
> +			memcpy(&vcpu_sys_reg(vcpu, DBGBVR0_EL1),
> +			       &regs->dbg_bvr,
> +			       sizeof(__u64)*nb);
> +			memcpy(&vcpu_sys_reg(vcpu, DBGWCR0_EL1),
> +			       &regs->dbg_wcr,
> +			       sizeof(__u64)*nw);
> +			memcpy(&vcpu_sys_reg(vcpu, DBGWVR0_EL1),
> +			       &regs->dbg_wvr,
> +			       sizeof(__u64)*nw);

same style comment as above

> +		}

this is called on iteration of the run-loop, right?

it really seems excessive to do all this copying on every single
iteration.  I think it's cleaner if you have on piece of state on the
vcpu struct for the guest state, and one piece of state for guest
debugging, and for all emulation and userspace sync purposes you access
the guest state directly and you switch a pointer to the struct that
EL2 uses to save/restore the underlying hardware state.  Would that
work?


>  	}
>  }
> 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
                                    set up
> + * 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.
> + *
>   */
> +
>  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);
> +
> +		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;
> +		}

you should be able to greatly simplify this as well if you follow my
suggestion above.

> +
> +		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 v2 08/10] KVM: arm64: guest debug, HW assisted debug support
From: Christoffer Dall @ 2015-04-14 10:23 UTC (permalink / raw)
  To: Andrew Jones
  Cc: Alex Bennée, kvm, linux-arm-kernel, kvmarm, marc.zyngier,
	peter.maydell, agraf, pbonzini, zhichao.huang, jan.kiszka, dahi,
	r65777, bp, Gleb Natapov, Jonathan Corbet, Russell King,
	Catalin Marinas, Will Deacon, AKASHI Takahiro, Lorenzo Pieralisi,
	open list:DOCUMENTATION, open list, open list:ABI/API
In-Reply-To: <20150410122520.GA3227@hawk.usersys.redhat.com>

On Fri, Apr 10, 2015 at 02:25:21PM +0200, Andrew Jones wrote:

[...]

> > --- 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

since I'm in this mood:

setup: noun or adjective
set-up: noun derived from the phrasal verb, example "Run! It's a set-up."
set up: verb

-Christoffer

^ permalink raw reply

* Re: [PATCH 0/5] Enhancements to twl4030 phy to support better charging - V2
From: Kishon Vijay Abraham I @ 2015-04-14 10:54 UTC (permalink / raw)
  To: NeilBrown
  Cc: NeilBrown, Tony Lindgren, linux-api-u79uwXL29TY76Z2rM5mHXA,
	GTA04 owners, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Pavel Machek,
	cw00.choi-Sze3O3UU22JBDgjK7y7TUQ,
	myungjoo.ham-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20150404112816.025d233b-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>

Hi,

On Saturday 04 April 2015 05:58 AM, NeilBrown wrote:
> On Fri, 3 Apr 2015 19:08:22 +0530 Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> wrote:
>
>> +Extcon MAINTAINERS
>>
>> Hi,
>>
>> On Wednesday 01 April 2015 10:11 AM, NeilBrown wrote:
>>> On Thu, 26 Mar 2015 05:29:42 +0530 Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>>> wrote:
>>>
>>>> Hi NeilBrown,
>>>>
>>>> On Thursday 26 March 2015 02:52 AM, NeilBrown wrote:
>>>>> On Thu, 26 Mar 2015 02:46:32 +0530 Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
>>>>> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Monday 23 March 2015 04:05 AM, NeilBrown wrote:
>>>>>>> Hi Kishon,
>>>>>>>    I wonder if you could queue the following for the next merge window.
>>>>>>>    They allow the twl4030 phy to provide more information to the
>>>>>>>    twl4030 battery charger.
>>>>>>>    There are only minimal changes since the first version, particularly
>>>>>>>    documentation has been improved.
>>>>>>
>>>>>> There are quite a few things in this series which use the USB PHY library
>>>>>> interface which is kindof deprecated. We should try and use the Generic PHY
>>>>>> library for all of them. It would also be better to add features to the
>>>>>> PHY framework if the we can't achieve something with the existing PHY
>>>>>> framework.
>>>>>
>>>>> Hi,
>>>>>    are you able to more specific at all?  What is the "USB PHY library"?
>>>>> Where exactly is the "PHY framework"?
>>>>
>>>> There is a USB PHY library that exists in drivers/usb/phy/phy.c and there is
>>>> a Generic PHY framework that is present in drivers/phy/phy-core.c. twl4030
>>>> actually supports both the framework.
>>>>
>>>> In your patch whatever uses struct usb_phy uses the old USB PHY library and
>>>> whatever uses struct phy uses the generic PHY framework. (Actually your patch
>>>> does not use the PHY framework at all). We want to deprecate using the USB PHY
>>>> library and make everyone use the generic PHY framework. Adding features
>>>> to a driver using the USB PHY library will make the transition to generic PHY
>>>> framework a bit more difficult.
>>>>
>>>> Now all the features that is supported in the USB PHY library may not be
>>>> supported by the PHY framework. So we should start extending the PHY framework
>>>> instead of using the USB PHY library.
>>>>
>>>> One think I noticed in your driver is using atomic notifier chain. IMO extcon
>>>> framework should be used in twl4030 USB driver to notify the controller driver
>>>> instead of using USB PHY notifier. For all other things we have to see if it
>>>> can be added in the PHY framework.
>>>
>>> I've had a look at the code with these issues in mind, and there is one issue
>>> that I'm not sure about.
>>>
>>> In phy-twl4030-usb, the usb_phy is used to hold a reference to the
>>> 'struct otg', and for passing cable state changes to the notifier.
>>
>> right now we directly call omap_musb_mailbox no? we don't use notifiers right?
>
> Correct, and omap_musb_set_mailbox uses the notifier chain.
> phy-twl4030-usb does
> 	ATOMIC_INIT_NOTIFIER_HEAD(&twl->phy.notifier);
> That is the only place the current phy code interacts with the notifier chain.

ah.. okay.
>
>
>>>
>>> The former probably has to stay until musb can keep a reference to the otg,
>>> separate form the usb_phy.  The latter can be changed to use extcon - to
>>> some extent.  I actually have patches to do that from a couple of years back,
>>> but I never proceeded with them.
>>>
>>> The problem is that one thing that needs to be communicated to the charger is
>>> the max current that was negotiated by a "Standard Downstream Port".
>>> This could be 500mA from a powered hum, or much less from an unpowered hub.
>>> (Currently the usb gadget code does negotiated between different
>>> possibilities, but it could and hopefully will one day).
>>>
>>> With the notifier chain there is an easy way to communicate the allowed
>>> current once it is negotiated. e.g. ab8500_usb_set_power() does this.
>>>
>>> 'struct phy' has no equivalent of the 'set_power' callback  which 'struct
>>> usb_phy' provides, and extcon has no mechanism (that I can see) for
>>> communicating a number - just binary cable states.
>>
>> Chanwoo Choi, Can this be modified so that we can communicate numbers like in
>> the case of EXTCON_CHARGE_DOWNSTREAM?
>>>
>>> Presumably a 'set_power' method could be added to 'struct phy' so the
>>> usb-core can communicate the number to the phy, but it is not clear to me how
>>> the 'phy' can communicate it to the charger.
>>
>> Should the PHY be involved in all this? We can make the gadget driver
>> directly communicate the value to the charger no?
>>> The 'phy' could provide an API to request the current negotiated max current,
>>> but there still needs to be a way to let the charger know that this has
>>> changed.
>>> That could in theory be done via extcon, by having a secondary
>>> 'USB_connected' cable type, but it isn't really a cable type and pretending
>>> that it is seems wrong.
>>
>> I think EXTCON_CHARGE_DOWNSTREAM was created for that purpose. Chanwoo?
>>
>
> EXTCON_CHARGE_DOWNSTREAM is something quite different.
>
> There are roughly three ways that the USB gadget can determine what sort of
> thing has been plugged in to it and what current it can draw.
>
>   - it can look at the resistance between the ID pin and GROUND.  This is a
>     physical property of the cable and it makes a lot of sense of EXTCON
>     to report different cables based on different resistances.
>
>   - it can look at the voltage provided on different pins.  If it detects a
>     certain voltage on D- when it asserts a voltage on D+, it can know
>     that it is a Charging Downstream Port (EXTCON_CHARGE_DOWNSTREAM).  This
>     might be a property of the cable (shorting D- to D+ can achieve this) or
>     might be a property of the attached device.  It makes some sense for
>     EXTCON to report cable type based on this sort of information.
>
>   - it can wait for the connected host to initiate a USB session and select a
>     particular profile.  That profile will include a "MaxPower" field.  When
>     the host selects that profile, the gadget knows it is allowed to draw that
>     much power ("current" really, measured in mA).

Thanks for that explanation :-)
>
> So EXTCON_CHARGE_DOWNSTREAM fits into the second category.  My question is
> about the third category.
> I need this "MaxPower" number to be communicated from the USB core to the
> charger driver, presumably via the "phy" driver.
>
> With "usb_phy", there is a ->set_power() callback to communicate from
> usb-core to phy, and a notifier chain to communicate from phy to charger.
> With "phy" there is nothing.

set_power sounds very specific to USB. Just thinking if we should make use of 
the regulator framework to set the current. With this the usb should create a 
dummy regulator and set the current and the charger can use the regulator.

Not sure if that makes sense though:-/

Thanks
Kishon

^ permalink raw reply

* [PATCH v8 1/3] scsi: ufs: add ioctl interface for query request
From: Gilad Broner @ 2015-04-14 11:51 UTC (permalink / raw)
  To: James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	santoshsy-Re5JQEeQqe8AvxtiuMwx3w,
	linux-scsi-owner-u79uwXL29TY76Z2rM5mHXA,
	subhashj-sgV2jX0FEOL9JmXXK+q4OQ, ygardi-sgV2jX0FEOL9JmXXK+q4OQ,
	draviv-sgV2jX0FEOL9JmXXK+q4OQ, Noa Rubens, Raviv Shvili,
	Gilad Broner, Vinayak Holikatti, James E.J. Bottomley,
	open list:ABI/API
In-Reply-To: <1429012281-24068-1-git-send-email-gbroner-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

From: Dolev Raviv <draviv-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

This patch exposes the ioctl interface for UFS driver via SCSI device
ioctl interface. As of now UFS driver would provide the ioctl for query
interface to connected UFS device.

Signed-off-by: Dolev Raviv <draviv-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Noa Rubens <noag-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Raviv Shvili <rshvili-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Yaniv Gardi <ygardi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Gilad Broner <gbroner-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/scsi/ufs/ufs.h        |  53 +++-------
 drivers/scsi/ufs/ufshcd.c     | 219 +++++++++++++++++++++++++++++++++++++++++-
 include/scsi/scsi.h           |   1 +
 include/uapi/scsi/Kbuild      |   1 +
 include/uapi/scsi/ufs/Kbuild  |   3 +
 include/uapi/scsi/ufs/ioctl.h |  57 +++++++++++
 include/uapi/scsi/ufs/ufs.h   |  66 +++++++++++++
 7 files changed, 356 insertions(+), 44 deletions(-)
 create mode 100644 include/uapi/scsi/ufs/Kbuild
 create mode 100644 include/uapi/scsi/ufs/ioctl.h
 create mode 100644 include/uapi/scsi/ufs/ufs.h

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 42c459a..1f023c4 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -38,6 +38,7 @@
 
 #include <linux/mutex.h>
 #include <linux/types.h>
+#include <scsi/ufs/ufs.h>
 
 #define MAX_CDB_SIZE	16
 #define GENERAL_UPIU_REQUEST_SIZE 32
@@ -71,6 +72,16 @@ enum {
 	UFS_UPIU_RPMB_WLUN		= 0xC4,
 };
 
+/**
+ * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit descriptor
+ * @lun: LU number to check
+ * @return: true if the lun has a matching unit descriptor, false otherwise
+ */
+static inline bool ufs_is_valid_unit_desc_lun(u8 lun)
+{
+	return (lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN));
+}
+
 /*
  * UFS Protocol Information Unit related definitions
  */
@@ -126,35 +137,6 @@ enum {
 	UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST          = 0x81,
 };
 
-/* Flag idn for Query Requests*/
-enum flag_idn {
-	QUERY_FLAG_IDN_FDEVICEINIT      = 0x01,
-	QUERY_FLAG_IDN_PWR_ON_WPE	= 0x03,
-	QUERY_FLAG_IDN_BKOPS_EN         = 0x04,
-};
-
-/* Attribute idn for Query requests */
-enum attr_idn {
-	QUERY_ATTR_IDN_ACTIVE_ICC_LVL	= 0x03,
-	QUERY_ATTR_IDN_BKOPS_STATUS	= 0x05,
-	QUERY_ATTR_IDN_EE_CONTROL	= 0x0D,
-	QUERY_ATTR_IDN_EE_STATUS	= 0x0E,
-};
-
-/* Descriptor idn for Query requests */
-enum desc_idn {
-	QUERY_DESC_IDN_DEVICE		= 0x0,
-	QUERY_DESC_IDN_CONFIGURAION	= 0x1,
-	QUERY_DESC_IDN_UNIT		= 0x2,
-	QUERY_DESC_IDN_RFU_0		= 0x3,
-	QUERY_DESC_IDN_INTERCONNECT	= 0x4,
-	QUERY_DESC_IDN_STRING		= 0x5,
-	QUERY_DESC_IDN_RFU_1		= 0x6,
-	QUERY_DESC_IDN_GEOMETRY		= 0x7,
-	QUERY_DESC_IDN_POWER		= 0x8,
-	QUERY_DESC_IDN_MAX,
-};
-
 enum desc_header_offset {
 	QUERY_DESC_LENGTH_OFFSET	= 0x00,
 	QUERY_DESC_DESC_TYPE_OFFSET	= 0x01,
@@ -247,19 +229,6 @@ enum bkops_status {
 	BKOPS_STATUS_MAX		 = BKOPS_STATUS_CRITICAL,
 };
 
-/* UTP QUERY Transaction Specific Fields OpCode */
-enum query_opcode {
-	UPIU_QUERY_OPCODE_NOP		= 0x0,
-	UPIU_QUERY_OPCODE_READ_DESC	= 0x1,
-	UPIU_QUERY_OPCODE_WRITE_DESC	= 0x2,
-	UPIU_QUERY_OPCODE_READ_ATTR	= 0x3,
-	UPIU_QUERY_OPCODE_WRITE_ATTR	= 0x4,
-	UPIU_QUERY_OPCODE_READ_FLAG	= 0x5,
-	UPIU_QUERY_OPCODE_SET_FLAG	= 0x6,
-	UPIU_QUERY_OPCODE_CLEAR_FLAG	= 0x7,
-	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
-};
-
 /* Query response result code */
 enum {
 	QUERY_RESULT_SUCCESS                    = 0x00,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index dfd72af..b6aa7fb 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3,7 +3,7 @@
  *
  * This code is based on drivers/scsi/ufs/ufshcd.c
  * Copyright (C) 2011-2013 Samsung India Software Operations
- * Copyright (c) 2013-2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013-2015, The Linux Foundation. All rights reserved.
  *
  * Authors:
  *	Santosh Yaraganavi <santosh.sy-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@@ -39,6 +39,7 @@
 
 #include <linux/async.h>
 #include <linux/devfreq.h>
+#include <scsi/ufs/ioctl.h>
 
 #include "ufshcd.h"
 #include "unipro.h"
@@ -74,6 +75,9 @@
 /* Interrupt aggregation default timeout, unit: 40us */
 #define INT_AGGR_DEF_TO	0x02
 
+/* IOCTL opcode for command - ufs set device read only */
+#define UFS_IOCTL_BLKROSET      BLKROSET
+
 #define ufshcd_toggle_vreg(_dev, _vreg, _on)				\
 	({                                                              \
 		int _ret;                                               \
@@ -1885,7 +1889,7 @@ static inline int ufshcd_read_unit_desc_param(struct ufs_hba *hba,
 	 * Unit descriptors are only available for general purpose LUs (LUN id
 	 * from 0 to 7) and RPMB Well known LU.
 	 */
-	if (lun != UFS_UPIU_RPMB_WLUN && (lun >= UFS_UPIU_MAX_GENERAL_LUN))
+	if (!ufs_is_valid_unit_desc_lun(lun))
 		return -EOPNOTSUPP;
 
 	return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun,
@@ -4236,6 +4240,216 @@ static void ufshcd_async_scan(void *data, async_cookie_t cookie)
 	ufshcd_probe_hba(hba);
 }
 
+/**
+ * ufshcd_query_ioctl - perform user read queries
+ * @hba: per-adapter instance
+ * @lun: used for lun specific queries
+ * @buffer: user space buffer for reading and submitting query data and params
+ * @return: 0 for success negative error code otherwise
+ *
+ * Expected/Submitted buffer structure is struct ufs_ioctl_query_data.
+ * It will read the opcode, idn and buf_length parameters, and, put the
+ * response in the buffer field while updating the used size in buf_length.
+ */
+static int ufshcd_query_ioctl(struct ufs_hba *hba, u8 lun, void __user *buffer)
+{
+	struct ufs_ioctl_query_data *ioctl_data;
+	int err = 0;
+	int length = 0;
+	void *data_ptr;
+	bool flag;
+	u32 att;
+	u8 index;
+	u8 *desc = NULL;
+
+	if (!buffer) {
+		dev_err(hba->dev, "%s: User buffer is NULL!\n", __func__);
+		return -EINVAL;
+	}
+
+	ioctl_data = kzalloc(sizeof(struct ufs_ioctl_query_data), GFP_KERNEL);
+	if (!ioctl_data) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	/* extract params from user buffer */
+	if (copy_from_user(ioctl_data, buffer, sizeof(*ioctl_data))) {
+		err = -EFAULT;
+		goto out_release_mem;
+	}
+
+	/* verify legal parameters & send query */
+	switch (ioctl_data->opcode) {
+	case UPIU_QUERY_OPCODE_READ_DESC:
+		switch (ioctl_data->idn) {
+		case QUERY_DESC_IDN_DEVICE:
+		case QUERY_DESC_IDN_CONFIGURAION:
+		case QUERY_DESC_IDN_INTERCONNECT:
+		case QUERY_DESC_IDN_GEOMETRY:
+		case QUERY_DESC_IDN_POWER:
+			index = 0;
+			break;
+		case QUERY_DESC_IDN_UNIT:
+			if (!ufs_is_valid_unit_desc_lun(lun)) {
+				dev_err(hba->dev,
+					"%s: No unit descriptor for lun 0x%x\n",
+					__func__, lun);
+				err = -EINVAL;
+				goto out_release_mem;
+			}
+			index = lun;
+			break;
+		default:
+			goto out_einval;
+		}
+		length = min_t(int, QUERY_DESC_MAX_SIZE,
+				ioctl_data->buf_size);
+		desc = kzalloc(length, GFP_KERNEL);
+		if (!desc) {
+			dev_err(hba->dev, "%s: Failed allocating %d bytes\n",
+					__func__, length);
+			err = -ENOMEM;
+			goto out_release_mem;
+		}
+		err = ufshcd_query_descriptor(hba, ioctl_data->opcode,
+				ioctl_data->idn, index, 0, desc, &length);
+		break;
+	case UPIU_QUERY_OPCODE_READ_ATTR:
+		switch (ioctl_data->idn) {
+		case QUERY_ATTR_IDN_BOOT_LU_EN:
+		case QUERY_ATTR_IDN_POWER_MODE:
+		case QUERY_ATTR_IDN_ACTIVE_ICC_LVL:
+		case QUERY_ATTR_IDN_OOO_DATA_EN:
+		case QUERY_ATTR_IDN_BKOPS_STATUS:
+		case QUERY_ATTR_IDN_PURGE_STATUS:
+		case QUERY_ATTR_IDN_MAX_DATA_IN:
+		case QUERY_ATTR_IDN_MAX_DATA_OUT:
+		case QUERY_ATTR_IDN_REF_CLK_FREQ:
+		case QUERY_ATTR_IDN_CONF_DESC_LOCK:
+		case QUERY_ATTR_IDN_MAX_NUM_OF_RTT:
+		case QUERY_ATTR_IDN_EE_CONTROL:
+		case QUERY_ATTR_IDN_EE_STATUS:
+		case QUERY_ATTR_IDN_SECONDS_PASSED:
+			index = 0;
+			break;
+		case QUERY_ATTR_IDN_DYN_CAP_NEEDED:
+		case QUERY_ATTR_IDN_CORR_PRG_BLK_NUM:
+			index = lun;
+			break;
+		default:
+			goto out_einval;
+		}
+		err = ufshcd_query_attr(hba, ioctl_data->opcode,
+					ioctl_data->idn, index, 0, &att);
+		break;
+	case UPIU_QUERY_OPCODE_READ_FLAG:
+		switch (ioctl_data->idn) {
+		case QUERY_FLAG_IDN_FDEVICEINIT:
+		case QUERY_FLAG_IDN_PERMANENT_WPE:
+		case QUERY_FLAG_IDN_PWR_ON_WPE:
+		case QUERY_FLAG_IDN_BKOPS_EN:
+		case QUERY_FLAG_IDN_PURGE_ENABLE:
+		case QUERY_FLAG_IDN_FPHYRESOURCEREMOVAL:
+		case QUERY_FLAG_IDN_BUSY_RTC:
+			break;
+		default:
+			goto out_einval;
+		}
+		err = ufshcd_query_flag(hba, ioctl_data->opcode,
+					ioctl_data->idn, &flag);
+		break;
+	default:
+		goto out_einval;
+	}
+
+	if (err) {
+		dev_err(hba->dev, "%s: Query for idn %d failed\n", __func__,
+					ioctl_data->idn);
+		goto out_release_mem;
+	}
+
+	/*
+	 * copy response data
+	 * As we might end up reading less data then what is specified in
+	 * "ioct_data->buf_size". So we are updating "ioct_data->
+	 * buf_size" to what exactly we have read.
+	 */
+	switch (ioctl_data->opcode) {
+	case UPIU_QUERY_OPCODE_READ_DESC:
+		ioctl_data->buf_size = min_t(int, ioctl_data->buf_size, length);
+		data_ptr = desc;
+		break;
+	case UPIU_QUERY_OPCODE_READ_ATTR:
+		ioctl_data->buf_size = sizeof(u32);
+		data_ptr = &att;
+		break;
+	case UPIU_QUERY_OPCODE_READ_FLAG:
+		ioctl_data->buf_size = 1;
+		data_ptr = &flag;
+		break;
+	default:
+		BUG_ON(true);
+	}
+
+	/* copy to user */
+	err = copy_to_user(buffer, ioctl_data, sizeof(*ioctl_data));
+	if (!err)
+		err = copy_to_user(buffer + sizeof(*ioctl_data), data_ptr,
+			ioctl_data->buf_size);
+	if (err)
+		err = -EFAULT;
+
+	goto out_release_mem;
+
+out_einval:
+	dev_err(hba->dev,
+		"%s: illegal ufs query ioctl data, opcode 0x%x, idn 0x%x\n",
+		__func__, ioctl_data->opcode, (unsigned int)ioctl_data->idn);
+	err = -EINVAL;
+out_release_mem:
+	kfree(ioctl_data);
+	kfree(desc);
+out:
+	return err;
+}
+
+/**
+ * ufshcd_ioctl - ufs ioctl callback registered in scsi_host
+ * @dev: scsi device required for per LUN queries
+ * @cmd: command opcode
+ * @buffer: user space buffer for transferring data
+ *
+ * Supported commands:
+ * UFS_IOCTL_QUERY
+ */
+static int ufshcd_ioctl(struct scsi_device *dev, int cmd, void __user *buffer)
+{
+	struct ufs_hba *hba = shost_priv(dev->host);
+	int err = 0;
+
+	BUG_ON(!hba);
+
+	switch (cmd) {
+	case UFS_IOCTL_QUERY:
+		pm_runtime_get_sync(hba->dev);
+		err = ufshcd_query_ioctl(hba, ufshcd_scsi_to_upiu_lun(dev->lun),
+				buffer);
+		pm_runtime_put_sync(hba->dev);
+		break;
+	case UFS_IOCTL_BLKROSET:
+		err = -ENOIOCTLCMD;
+		break;
+	default:
+		err = -EINVAL;
+		dev_err(hba->dev, "%s: Illegal ufs-IOCTL cmd %d\n", __func__,
+				cmd);
+		break;
+	}
+
+	return err;
+}
+
 static struct scsi_host_template ufshcd_driver_template = {
 	.module			= THIS_MODULE,
 	.name			= UFSHCD,
@@ -4248,6 +4462,7 @@ static struct scsi_host_template ufshcd_driver_template = {
 	.eh_abort_handler	= ufshcd_abort,
 	.eh_device_reset_handler = ufshcd_eh_device_reset_handler,
 	.eh_host_reset_handler   = ufshcd_eh_host_reset_handler,
+	.ioctl			= ufshcd_ioctl,
 	.this_id		= -1,
 	.sg_tablesize		= SG_ALL,
 	.cmd_per_lun		= UFSHCD_CMD_PER_LUN,
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index d0a66aa..3feb501 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -569,6 +569,7 @@ static inline int scsi_is_wlun(u64 lun)
  * Here are some scsi specific ioctl commands which are sometimes useful.
  *
  * Note that include/linux/cdrom.h also defines IOCTL 0x5300 - 0x5395
+ * include/uapi/scsi/ufs/ioctl.h defines 0x5388
  */
 
 /* Used to obtain PUN and LUN info.  Conflicts with CDROMAUDIOBUFSIZ */
diff --git a/include/uapi/scsi/Kbuild b/include/uapi/scsi/Kbuild
index 75746d5..d404525 100644
--- a/include/uapi/scsi/Kbuild
+++ b/include/uapi/scsi/Kbuild
@@ -1,5 +1,6 @@
 # UAPI Header export list
 header-y += fc/
+header-y += ufs/
 header-y += scsi_bsg_fc.h
 header-y += scsi_netlink.h
 header-y += scsi_netlink_fc.h
diff --git a/include/uapi/scsi/ufs/Kbuild b/include/uapi/scsi/ufs/Kbuild
new file mode 100644
index 0000000..cc3ef20
--- /dev/null
+++ b/include/uapi/scsi/ufs/Kbuild
@@ -0,0 +1,3 @@
+# UAPI Header export list
+header-y += ioctl.h
+header-y += ufs.h
diff --git a/include/uapi/scsi/ufs/ioctl.h b/include/uapi/scsi/ufs/ioctl.h
new file mode 100644
index 0000000..bc4eed7
--- /dev/null
+++ b/include/uapi/scsi/ufs/ioctl.h
@@ -0,0 +1,57 @@
+#ifndef UAPI_UFS_IOCTL_H_
+#define UAPI_UFS_IOCTL_H_
+
+#include <linux/types.h>
+
+/*
+ *  IOCTL opcode for ufs queries has the following opcode after
+ *  SCSI_IOCTL_GET_PCI
+ */
+#define UFS_IOCTL_QUERY			0x5388
+
+/**
+ * struct ufs_ioctl_query_data - used to transfer data to and from user via ioctl
+ * @opcode: type of data to query (descriptor/attribute/flag)
+ * @idn: id of the data structure
+ * @buf_size: number of allocated bytes/data size on return
+ * @buffer: data location
+ *
+ * Received: buffer and buf_size (available space for transferred data)
+ * Submitted: opcode, idn, length, buf_size
+ */
+struct ufs_ioctl_query_data {
+	/*
+	 * User should select one of the opcode defined in "enum query_opcode".
+	 * Please check include/uapi/scsi/ufs/ufs.h for the definition of it.
+	 * Note that only UPIU_QUERY_OPCODE_READ_DESC,
+	 * UPIU_QUERY_OPCODE_READ_ATTR & UPIU_QUERY_OPCODE_READ_FLAG are
+	 * supported as of now. All other query_opcode would be considered
+	 * invalid.
+	 * As of now only read query operations are supported.
+	 */
+	__u32 opcode;
+	/*
+	 * User should select one of the idn from "enum flag_idn" or "enum
+	 * attr_idn" or "enum desc_idn" based on whether opcode above is
+	 * attribute, flag or descriptor.
+	 * Please check include/uapi/scsi/ufs/ufs.h for the definition of it.
+	 */
+	__u8 idn;
+	/*
+	 * User should specify the size of the buffer (buffer[0] below) where
+	 * it wants to read the query data (attribute/flag/descriptor).
+	 * As we might end up reading less data then what is specified in
+	 * buf_size. So we are updating buf_size to what exactly we have read.
+	 */
+	__u16 buf_size;
+	/*
+	 * placeholder for the start of the data buffer where kernel will copy
+	 * the query data (attribute/flag/descriptor) read from the UFS device
+	 * Note:
+	 * For Read Attribute you will have to allocate 4 bytes
+	 * For Read Flag you will have to allocate 1 byte
+	 */
+	__u8 buffer[0];
+};
+
+#endif /* UAPI_UFS_IOCTL_H_ */
diff --git a/include/uapi/scsi/ufs/ufs.h b/include/uapi/scsi/ufs/ufs.h
new file mode 100644
index 0000000..894ea45
--- /dev/null
+++ b/include/uapi/scsi/ufs/ufs.h
@@ -0,0 +1,66 @@
+#ifndef UAPI_UFS_H_
+#define UAPI_UFS_H_
+
+/* Flag idn for Query Requests*/
+enum flag_idn {
+	QUERY_FLAG_IDN_FDEVICEINIT		= 0x01,
+	QUERY_FLAG_IDN_PERMANENT_WPE		= 0x02,
+	QUERY_FLAG_IDN_PWR_ON_WPE		= 0x03,
+	QUERY_FLAG_IDN_BKOPS_EN			= 0x04,
+	QUERY_FLAG_IDN_RESERVED1		= 0x05,
+	QUERY_FLAG_IDN_PURGE_ENABLE		= 0x06,
+	QUERY_FLAG_IDN_RESERVED2		= 0x07,
+	QUERY_FLAG_IDN_FPHYRESOURCEREMOVAL      = 0x08,
+	QUERY_FLAG_IDN_BUSY_RTC			= 0x09,
+};
+
+/* Attribute idn for Query requests */
+enum attr_idn {
+	QUERY_ATTR_IDN_BOOT_LU_EN		= 0x00,
+	QUERY_ATTR_IDN_RESERVED			= 0x01,
+	QUERY_ATTR_IDN_POWER_MODE		= 0x02,
+	QUERY_ATTR_IDN_ACTIVE_ICC_LVL		= 0x03,
+	QUERY_ATTR_IDN_OOO_DATA_EN		= 0x04,
+	QUERY_ATTR_IDN_BKOPS_STATUS		= 0x05,
+	QUERY_ATTR_IDN_PURGE_STATUS		= 0x06,
+	QUERY_ATTR_IDN_MAX_DATA_IN		= 0x07,
+	QUERY_ATTR_IDN_MAX_DATA_OUT		= 0x08,
+	QUERY_ATTR_IDN_DYN_CAP_NEEDED		= 0x09,
+	QUERY_ATTR_IDN_REF_CLK_FREQ		= 0x0A,
+	QUERY_ATTR_IDN_CONF_DESC_LOCK		= 0x0B,
+	QUERY_ATTR_IDN_MAX_NUM_OF_RTT		= 0x0C,
+	QUERY_ATTR_IDN_EE_CONTROL		= 0x0D,
+	QUERY_ATTR_IDN_EE_STATUS		= 0x0E,
+	QUERY_ATTR_IDN_SECONDS_PASSED		= 0x0F,
+	QUERY_ATTR_IDN_CNTX_CONF		= 0x10,
+	QUERY_ATTR_IDN_CORR_PRG_BLK_NUM		= 0x11,
+};
+
+/* Descriptor idn for Query requests */
+enum desc_idn {
+	QUERY_DESC_IDN_DEVICE		= 0x0,
+	QUERY_DESC_IDN_CONFIGURAION	= 0x1,
+	QUERY_DESC_IDN_UNIT		= 0x2,
+	QUERY_DESC_IDN_RFU_0		= 0x3,
+	QUERY_DESC_IDN_INTERCONNECT	= 0x4,
+	QUERY_DESC_IDN_STRING		= 0x5,
+	QUERY_DESC_IDN_RFU_1		= 0x6,
+	QUERY_DESC_IDN_GEOMETRY		= 0x7,
+	QUERY_DESC_IDN_POWER		= 0x8,
+	QUERY_DESC_IDN_RFU_2		= 0x9,
+	QUERY_DESC_IDN_MAX,
+};
+
+/* UTP QUERY Transaction Specific Fields OpCode */
+enum query_opcode {
+	UPIU_QUERY_OPCODE_NOP		= 0x0,
+	UPIU_QUERY_OPCODE_READ_DESC	= 0x1,
+	UPIU_QUERY_OPCODE_WRITE_DESC	= 0x2,
+	UPIU_QUERY_OPCODE_READ_ATTR	= 0x3,
+	UPIU_QUERY_OPCODE_WRITE_ATTR	= 0x4,
+	UPIU_QUERY_OPCODE_READ_FLAG	= 0x5,
+	UPIU_QUERY_OPCODE_SET_FLAG	= 0x6,
+	UPIU_QUERY_OPCODE_CLEAR_FLAG	= 0x7,
+	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
+};
+#endif /* UAPI_UFS_H_ */
-- 
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox