All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
@ 2019-01-09  4:40 kbuild test robot
  2019-01-09  9:13   ` André Przywara
  0 siblings, 1 reply; 9+ messages in thread
From: kbuild test robot @ 2019-01-09  4:40 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Marc Zyngier, Christoffer Dall, kbuild-all, linux-arm-kernel, kvmarm

[-- Attachment #1: Type: text/plain, Size: 5997 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git kvm-arm64/nv-wip-v5.0-rc1
head:   688c386ca096f2c1f2eee386697586c88df5d5bc
commit: 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64: consolidate arch timer trap handlers
config: arm-axm55xx_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 2b1265c58a873d917e99ac762e243c1274481dbf
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'get_timer_from_sysreg':
>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
     case SYS_CNTP_TVAL_EL0:
          ^~~~~~~~~~~~~~~~~
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: note: each undeclared identifier is reported only once for each function it appears in
>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:701:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
     case SYS_CNTP_CTL_EL0:
          ^~~~~~~~~~~~~~~~
          SYS_CNTP_TVAL_EL0
>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:702:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
     case SYS_CNTP_CVAL_EL0:
          ^~~~~~~~~~~~~~~~~
          SYS_CNTP_TVAL_EL0
>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:703:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
     case SYS_AARCH32_CNTP_TVAL:
          ^~~~~~~~~~~~~~~~~~~~~
>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:704:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
     case SYS_AARCH32_CNTP_CTL:
          ^~~~~~~~~~~~~~~~~~~~
          SYS_AARCH32_CNTP_TVAL
>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:705:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
     case SYS_AARCH32_CNTP_CVAL:
          ^~~~~~~~~~~~~~~~~~~~~
          SYS_AARCH32_CNTP_TVAL
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'kvm_arm_timer_read_sysreg':
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:723:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
     case SYS_CNTP_TVAL_EL0:
          ^~~~~~~~~~~~~~~~~
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:724:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
     case SYS_AARCH32_CNTP_TVAL:
          ^~~~~~~~~~~~~~~~~~~~~
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:728:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
     case SYS_CNTP_CTL_EL0:
          ^~~~~~~~~~~~~~~~
          SYS_CNTP_TVAL_EL0
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:729:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
     case SYS_AARCH32_CNTP_CTL:
          ^~~~~~~~~~~~~~~~~~~~
          SYS_AARCH32_CNTP_TVAL
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:733:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
     case SYS_CNTP_CVAL_EL0:
          ^~~~~~~~~~~~~~~~~
          SYS_CNTP_TVAL_EL0
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:734:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
     case SYS_AARCH32_CNTP_CVAL:
          ^~~~~~~~~~~~~~~~~~~~~
          SYS_AARCH32_CNTP_TVAL
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'kvm_arm_timer_write_sysreg':
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:758:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
     case SYS_CNTP_TVAL_EL0:
          ^~~~~~~~~~~~~~~~~
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:759:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
     case SYS_AARCH32_CNTP_TVAL:
          ^~~~~~~~~~~~~~~~~~~~~
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:763:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
     case SYS_CNTP_CTL_EL0:
          ^~~~~~~~~~~~~~~~
          SYS_CNTP_TVAL_EL0
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:764:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
     case SYS_AARCH32_CNTP_CTL:
          ^~~~~~~~~~~~~~~~~~~~
          SYS_AARCH32_CNTP_TVAL
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:768:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
     case SYS_CNTP_CVAL_EL0:
          ^~~~~~~~~~~~~~~~~
          SYS_CNTP_TVAL_EL0
   arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:769:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
     case SYS_AARCH32_CNTP_CVAL:
          ^~~~~~~~~~~~~~~~~~~~~
          SYS_AARCH32_CNTP_TVAL

vim +/SYS_CNTP_TVAL_EL0 +700 arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c

   695	
   696	static struct arch_timer_context *get_timer_from_sysreg(struct kvm_vcpu *vcpu,
   697								u32 sr)
   698	{
   699		switch (sr) {
 > 700		case SYS_CNTP_TVAL_EL0:
 > 701		case SYS_CNTP_CTL_EL0:
 > 702		case SYS_CNTP_CVAL_EL0:
 > 703		case SYS_AARCH32_CNTP_TVAL:
 > 704		case SYS_AARCH32_CNTP_CTL:
 > 705		case SYS_AARCH32_CNTP_CVAL:
   706			return vcpu_ptimer(vcpu);
   707		default:
   708			BUG();
   709		}
   710	}
   711	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 20426 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
  2019-01-09  4:40 [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared kbuild test robot
@ 2019-01-09  9:13   ` André Przywara
  0 siblings, 0 replies; 9+ messages in thread
From: André Przywara @ 2019-01-09  9:13 UTC (permalink / raw)
  To: Marc Zyngier, Christoffer Dall; +Cc: kvmarm, linux-arm-kernel

On 09/01/2019 04:40, kbuild test robot wrote:

Marc, Christoffer,

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git kvm-arm64/nv-wip-v5.0-rc1
> head:   688c386ca096f2c1f2eee386697586c88df5d5bc
> commit: 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64: consolidate arch timer trap handlers
> config: arm-axm55xx_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         git checkout 2b1265c58a873d917e99ac762e243c1274481dbf
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.2.0 make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):

I was looking at this yesterday: It's a bit nasty, don't know a good
solution beside bringing back this part of my original timer rework series.
The problem is that those symbols contains the Aarch64 specific
(instruction) encoding of the timer registers, plus we need the AArch32
encodings for 32-on-64 guests.

That's why I used the generic UAPI encoding for the registers, because
we only need *some* identification for them, it doesn't need to be
something defined by the architecture.
Since arch_timer.c is shared, but sys_regs.c and coproc.c are arch
specific already, it seems natural to do the conversion in the trap
handler and keep the arch_timer code generic.

Does that make sense? I was a bit hesitant as it works against some of
your changes.

Cheers,
Andre.

> 
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'get_timer_from_sysreg':
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
>      case SYS_CNTP_TVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: note: each undeclared identifier is reported only once for each function it appears in
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:701:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CTL_EL0:
>           ^~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:702:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:703:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
>      case SYS_AARCH32_CNTP_TVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:704:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CTL:
>           ^~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:705:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'kvm_arm_timer_read_sysreg':
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:723:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
>      case SYS_CNTP_TVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:724:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
>      case SYS_AARCH32_CNTP_TVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:728:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CTL_EL0:
>           ^~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:729:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CTL:
>           ^~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:733:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:734:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'kvm_arm_timer_write_sysreg':
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:758:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
>      case SYS_CNTP_TVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:759:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
>      case SYS_AARCH32_CNTP_TVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:763:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CTL_EL0:
>           ^~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:764:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CTL:
>           ^~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:768:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:769:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
> 
> vim +/SYS_CNTP_TVAL_EL0 +700 arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c
> 
>    695	
>    696	static struct arch_timer_context *get_timer_from_sysreg(struct kvm_vcpu *vcpu,
>    697								u32 sr)
>    698	{
>    699		switch (sr) {
>  > 700		case SYS_CNTP_TVAL_EL0:
>  > 701		case SYS_CNTP_CTL_EL0:
>  > 702		case SYS_CNTP_CVAL_EL0:
>  > 703		case SYS_AARCH32_CNTP_TVAL:
>  > 704		case SYS_AARCH32_CNTP_CTL:
>  > 705		case SYS_AARCH32_CNTP_CVAL:
>    706			return vcpu_ptimer(vcpu);
>    707		default:
>    708			BUG();
>    709		}
>    710	}
>    711	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
@ 2019-01-09  9:13   ` André Przywara
  0 siblings, 0 replies; 9+ messages in thread
From: André Przywara @ 2019-01-09  9:13 UTC (permalink / raw)
  To: Marc Zyngier, Christoffer Dall; +Cc: kvmarm, linux-arm-kernel

On 09/01/2019 04:40, kbuild test robot wrote:

Marc, Christoffer,

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git kvm-arm64/nv-wip-v5.0-rc1
> head:   688c386ca096f2c1f2eee386697586c88df5d5bc
> commit: 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64: consolidate arch timer trap handlers
> config: arm-axm55xx_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         git checkout 2b1265c58a873d917e99ac762e243c1274481dbf
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.2.0 make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):

I was looking at this yesterday: It's a bit nasty, don't know a good
solution beside bringing back this part of my original timer rework series.
The problem is that those symbols contains the Aarch64 specific
(instruction) encoding of the timer registers, plus we need the AArch32
encodings for 32-on-64 guests.

That's why I used the generic UAPI encoding for the registers, because
we only need *some* identification for them, it doesn't need to be
something defined by the architecture.
Since arch_timer.c is shared, but sys_regs.c and coproc.c are arch
specific already, it seems natural to do the conversion in the trap
handler and keep the arch_timer code generic.

Does that make sense? I was a bit hesitant as it works against some of
your changes.

Cheers,
Andre.

> 
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'get_timer_from_sysreg':
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
>      case SYS_CNTP_TVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: note: each undeclared identifier is reported only once for each function it appears in
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:701:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CTL_EL0:
>           ^~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:702:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:703:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
>      case SYS_AARCH32_CNTP_TVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:704:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CTL:
>           ^~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>>> arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:705:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'kvm_arm_timer_read_sysreg':
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:723:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
>      case SYS_CNTP_TVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:724:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
>      case SYS_AARCH32_CNTP_TVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:728:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CTL_EL0:
>           ^~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:729:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CTL:
>           ^~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:733:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:734:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c: In function 'kvm_arm_timer_write_sysreg':
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:758:7: error: 'SYS_CNTP_TVAL_EL0' undeclared (first use in this function)
>      case SYS_CNTP_TVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:759:7: error: 'SYS_AARCH32_CNTP_TVAL' undeclared (first use in this function)
>      case SYS_AARCH32_CNTP_TVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:763:7: error: 'SYS_CNTP_CTL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CTL_EL0:
>           ^~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:764:7: error: 'SYS_AARCH32_CNTP_CTL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CTL:
>           ^~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:768:7: error: 'SYS_CNTP_CVAL_EL0' undeclared (first use in this function); did you mean 'SYS_CNTP_TVAL_EL0'?
>      case SYS_CNTP_CVAL_EL0:
>           ^~~~~~~~~~~~~~~~~
>           SYS_CNTP_TVAL_EL0
>    arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:769:7: error: 'SYS_AARCH32_CNTP_CVAL' undeclared (first use in this function); did you mean 'SYS_AARCH32_CNTP_TVAL'?
>      case SYS_AARCH32_CNTP_CVAL:
>           ^~~~~~~~~~~~~~~~~~~~~
>           SYS_AARCH32_CNTP_TVAL
> 
> vim +/SYS_CNTP_TVAL_EL0 +700 arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c
> 
>    695	
>    696	static struct arch_timer_context *get_timer_from_sysreg(struct kvm_vcpu *vcpu,
>    697								u32 sr)
>    698	{
>    699		switch (sr) {
>  > 700		case SYS_CNTP_TVAL_EL0:
>  > 701		case SYS_CNTP_CTL_EL0:
>  > 702		case SYS_CNTP_CVAL_EL0:
>  > 703		case SYS_AARCH32_CNTP_TVAL:
>  > 704		case SYS_AARCH32_CNTP_CTL:
>  > 705		case SYS_AARCH32_CNTP_CVAL:
>    706			return vcpu_ptimer(vcpu);
>    707		default:
>    708			BUG();
>    709		}
>    710	}
>    711	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
  2019-01-09  9:13   ` André Przywara
@ 2019-01-09 10:09     ` Marc Zyngier
  -1 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2019-01-09 10:09 UTC (permalink / raw)
  To: André Przywara, Christoffer Dall; +Cc: kvmarm, linux-arm-kernel

On 09/01/2019 09:13, André Przywara wrote:
> On 09/01/2019 04:40, kbuild test robot wrote:
> 
> Marc, Christoffer,
> 
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git kvm-arm64/nv-wip-v5.0-rc1
>> head:   688c386ca096f2c1f2eee386697586c88df5d5bc
>> commit: 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64: consolidate arch timer trap handlers
>> config: arm-axm55xx_defconfig (attached as .config)
>> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
>> reproduce:
>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         git checkout 2b1265c58a873d917e99ac762e243c1274481dbf
>>         # save the attached .config to linux build tree
>>         GCC_VERSION=7.2.0 make.cross ARCH=arm 
>>
>> All errors (new ones prefixed by >>):
> 
> I was looking at this yesterday: It's a bit nasty, don't know a good
> solution beside bringing back this part of my original timer rework series.
> The problem is that those symbols contains the Aarch64 specific
> (instruction) encoding of the timer registers, plus we need the AArch32
> encodings for 32-on-64 guests.

Why? There is exactly one timer that needs trapping for AArch32 (the EL1
physical timer). All we need is:

- the SYS_AARCH32_CNTP_* encodings on 32bit
- some CPP magic to prevent the compilation from breaking

> 
> That's why I used the generic UAPI encoding for the registers, because
> we only need *some* identification for them, it doesn't need to be
> something defined by the architecture.

I disagree. By doing so, you're conflating userspace access and
trapping, which has proved to be a bad idea in the past. For example,
you'd end-up having both CVAL and TVAL in UAPI, which is not something
I'm keen to have. On the other hand, the trapping function do need to be
able to handle these.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
@ 2019-01-09 10:09     ` Marc Zyngier
  0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2019-01-09 10:09 UTC (permalink / raw)
  To: André Przywara, Christoffer Dall; +Cc: kvmarm, linux-arm-kernel

On 09/01/2019 09:13, André Przywara wrote:
> On 09/01/2019 04:40, kbuild test robot wrote:
> 
> Marc, Christoffer,
> 
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git kvm-arm64/nv-wip-v5.0-rc1
>> head:   688c386ca096f2c1f2eee386697586c88df5d5bc
>> commit: 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64: consolidate arch timer trap handlers
>> config: arm-axm55xx_defconfig (attached as .config)
>> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
>> reproduce:
>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         git checkout 2b1265c58a873d917e99ac762e243c1274481dbf
>>         # save the attached .config to linux build tree
>>         GCC_VERSION=7.2.0 make.cross ARCH=arm 
>>
>> All errors (new ones prefixed by >>):
> 
> I was looking at this yesterday: It's a bit nasty, don't know a good
> solution beside bringing back this part of my original timer rework series.
> The problem is that those symbols contains the Aarch64 specific
> (instruction) encoding of the timer registers, plus we need the AArch32
> encodings for 32-on-64 guests.

Why? There is exactly one timer that needs trapping for AArch32 (the EL1
physical timer). All we need is:

- the SYS_AARCH32_CNTP_* encodings on 32bit
- some CPP magic to prevent the compilation from breaking

> 
> That's why I used the generic UAPI encoding for the registers, because
> we only need *some* identification for them, it doesn't need to be
> something defined by the architecture.

I disagree. By doing so, you're conflating userspace access and
trapping, which has proved to be a bad idea in the past. For example,
you'd end-up having both CVAL and TVAL in UAPI, which is not something
I'm keen to have. On the other hand, the trapping function do need to be
able to handle these.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
  2019-01-09 10:09     ` Marc Zyngier
@ 2019-01-09 10:43       ` Andre Przywara
  -1 siblings, 0 replies; 9+ messages in thread
From: Andre Przywara @ 2019-01-09 10:43 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, kvmarm

On Wed, 9 Jan 2019 10:09:51 +0000
Marc Zyngier <marc.zyngier@arm.com> wrote:

> On 09/01/2019 09:13, André Przywara wrote:
> > On 09/01/2019 04:40, kbuild test robot wrote:
> > 
> > Marc, Christoffer,
> >   
> >> tree:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git
> >> kvm-arm64/nv-wip-v5.0-rc1 head:
> >> 688c386ca096f2c1f2eee386697586c88df5d5bc commit:
> >> 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64:
> >> consolidate arch timer trap handlers config: arm-axm55xx_defconfig
> >> (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian
> >> 7.2.0-11) 7.2.0 reproduce: wget
> >> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> >> -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout
> >> 2b1265c58a873d917e99ac762e243c1274481dbf # save the
> >> attached .config to linux build tree GCC_VERSION=7.2.0 make.cross
> >> ARCH=arm 
> >>
> >> All errors (new ones prefixed by >>):  
> > 
> > I was looking at this yesterday: It's a bit nasty, don't know a good
> > solution beside bringing back this part of my original timer rework
> > series. The problem is that those symbols contains the Aarch64
> > specific (instruction) encoding of the timer registers, plus we
> > need the AArch32 encodings for 32-on-64 guests.  
> 
> Why? There is exactly one timer that needs trapping for AArch32 (the
> EL1 physical timer). All we need is:
> 
> - the SYS_AARCH32_CNTP_* encodings on 32bit
> - some CPP magic to prevent the compilation from breaking

Fair enough, if you are OK with this. I had something like this before,
but it looked kind of hackish to me, so I was looking at some proper(TM)
solution.

> > 
> > That's why I used the generic UAPI encoding for the registers,
> > because we only need *some* identification for them, it doesn't
> > need to be something defined by the architecture.  
> 
> I disagree. By doing so, you're conflating userspace access and
> trapping, which has proved to be a bad idea in the past. For example,
> you'd end-up having both CVAL and TVAL in UAPI, which is not something
> I'm keen to have. On the other hand, the trapping function do need to
> be able to handle these.

Fair enough, I was just *hoping* we can keep arch_timer.c clean of arch
specific encodings. I can understand that with all the added
complexity of the nested timers this is probably not feasible anymore.

Cheers,
Andre.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
@ 2019-01-09 10:43       ` Andre Przywara
  0 siblings, 0 replies; 9+ messages in thread
From: Andre Przywara @ 2019-01-09 10:43 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Christoffer Dall, linux-arm-kernel, kvmarm

On Wed, 9 Jan 2019 10:09:51 +0000
Marc Zyngier <marc.zyngier@arm.com> wrote:

> On 09/01/2019 09:13, André Przywara wrote:
> > On 09/01/2019 04:40, kbuild test robot wrote:
> > 
> > Marc, Christoffer,
> >   
> >> tree:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git
> >> kvm-arm64/nv-wip-v5.0-rc1 head:
> >> 688c386ca096f2c1f2eee386697586c88df5d5bc commit:
> >> 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64:
> >> consolidate arch timer trap handlers config: arm-axm55xx_defconfig
> >> (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian
> >> 7.2.0-11) 7.2.0 reproduce: wget
> >> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> >> -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout
> >> 2b1265c58a873d917e99ac762e243c1274481dbf # save the
> >> attached .config to linux build tree GCC_VERSION=7.2.0 make.cross
> >> ARCH=arm 
> >>
> >> All errors (new ones prefixed by >>):  
> > 
> > I was looking at this yesterday: It's a bit nasty, don't know a good
> > solution beside bringing back this part of my original timer rework
> > series. The problem is that those symbols contains the Aarch64
> > specific (instruction) encoding of the timer registers, plus we
> > need the AArch32 encodings for 32-on-64 guests.  
> 
> Why? There is exactly one timer that needs trapping for AArch32 (the
> EL1 physical timer). All we need is:
> 
> - the SYS_AARCH32_CNTP_* encodings on 32bit
> - some CPP magic to prevent the compilation from breaking

Fair enough, if you are OK with this. I had something like this before,
but it looked kind of hackish to me, so I was looking at some proper(TM)
solution.

> > 
> > That's why I used the generic UAPI encoding for the registers,
> > because we only need *some* identification for them, it doesn't
> > need to be something defined by the architecture.  
> 
> I disagree. By doing so, you're conflating userspace access and
> trapping, which has proved to be a bad idea in the past. For example,
> you'd end-up having both CVAL and TVAL in UAPI, which is not something
> I'm keen to have. On the other hand, the trapping function do need to
> be able to handle these.

Fair enough, I was just *hoping* we can keep arch_timer.c clean of arch
specific encodings. I can understand that with all the added
complexity of the nested timers this is probably not feasible anymore.

Cheers,
Andre.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
  2019-01-09 10:09     ` Marc Zyngier
@ 2019-01-09 10:48       ` Christoffer Dall
  -1 siblings, 0 replies; 9+ messages in thread
From: Christoffer Dall @ 2019-01-09 10:48 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: André Przywara, kvmarm, linux-arm-kernel

On Wed, Jan 09, 2019 at 10:09:51AM +0000, Marc Zyngier wrote:
> On 09/01/2019 09:13, André Przywara wrote:
> > On 09/01/2019 04:40, kbuild test robot wrote:
> > 
> > Marc, Christoffer,
> > 
> >> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git kvm-arm64/nv-wip-v5.0-rc1
> >> head:   688c386ca096f2c1f2eee386697586c88df5d5bc
> >> commit: 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64: consolidate arch timer trap handlers
> >> config: arm-axm55xx_defconfig (attached as .config)
> >> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> >> reproduce:
> >>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >>         chmod +x ~/bin/make.cross
> >>         git checkout 2b1265c58a873d917e99ac762e243c1274481dbf
> >>         # save the attached .config to linux build tree
> >>         GCC_VERSION=7.2.0 make.cross ARCH=arm 
> >>
> >> All errors (new ones prefixed by >>):
> > 
> > I was looking at this yesterday: It's a bit nasty, don't know a good
> > solution beside bringing back this part of my original timer rework series.
> > The problem is that those symbols contains the Aarch64 specific
> > (instruction) encoding of the timer registers, plus we need the AArch32
> > encodings for 32-on-64 guests.
> 
> Why? There is exactly one timer that needs trapping for AArch32 (the EL1
> physical timer). All we need is:
> 
> - the SYS_AARCH32_CNTP_* encodings on 32bit
> - some CPP magic to prevent the compilation from breaking
> 
> > 
> > That's why I used the generic UAPI encoding for the registers, because
> > we only need *some* identification for them, it doesn't need to be
> > something defined by the architecture.
> 
> I disagree. By doing so, you're conflating userspace access and
> trapping, which has proved to be a bad idea in the past. For example,
> you'd end-up having both CVAL and TVAL in UAPI, which is not something
> I'm keen to have. On the other hand, the trapping function do need to be
> able to handle these.
> 
I think it probably makes sense to have the sysreg encoding stuff in the
arch-specific files, and have an indirection in sys_regs.c and coproc.c
which 'translates' from a system register to some generic arch timer
define identifying a 'timer'.

I think the major breakage in the previous design was to use the same
*functions* for uaccess and for sysregs traps, but I don't think it was
necessarily a problem to use the same definitions to identify a timer.

That of couse leaves the problem of how to identify a register within a
timer.  Could we do something as braindead as:


diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index ce441dda412c..1d7bd452e5fd 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -32,6 +32,13 @@ enum kvm_arch_timers {
 	NR_KVM_TIMERS
 };
 
+enum arch_timer_reg {
+	ARCH_TIMER_REG_CTL,
+	ARCH_TIMER_REG_CVAL,
+	ARCH_TIMER_REG_TVAL,
+	ARCH_TIMER_REG_CNT
+}
+
 struct arch_timer_context {
 	struct kvm_vcpu			*vcpu;
 
@@ -86,8 +93,13 @@ bool kvm_timer_should_notify_user(struct kvm_vcpu *vcpu);
 void kvm_timer_update_run(struct kvm_vcpu *vcpu);
 void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu);
 
-u64 kvm_arm_timer_get_reg(struct kvm_vcpu *, u64 regid);
-int kvm_arm_timer_set_reg(struct kvm_vcpu *, u64 regid, u64 value);
+u64 kvm_arm_timer_get_reg(struct kvm_vcpu *,
+			  enum kvm_arch_timers timer,
+			  enum arch_timer_reg reg);
+int kvm_arm_timer_set_reg(struct kvm_vcpu *,
+			  enum kvm_arch_timers timer,
+			  enum arch_timer_reg reg,
+			  u64 value);
 
 u64 kvm_arm_timer_read_sysreg(struct kvm_vcpu *vcpu, u32 sr);
 void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu, u32 sr, u64 val);


Thanks,

    Christoffer
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared
@ 2019-01-09 10:48       ` Christoffer Dall
  0 siblings, 0 replies; 9+ messages in thread
From: Christoffer Dall @ 2019-01-09 10:48 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: André Przywara, kvmarm, linux-arm-kernel

On Wed, Jan 09, 2019 at 10:09:51AM +0000, Marc Zyngier wrote:
> On 09/01/2019 09:13, André Przywara wrote:
> > On 09/01/2019 04:40, kbuild test robot wrote:
> > 
> > Marc, Christoffer,
> > 
> >> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git kvm-arm64/nv-wip-v5.0-rc1
> >> head:   688c386ca096f2c1f2eee386697586c88df5d5bc
> >> commit: 2b1265c58a873d917e99ac762e243c1274481dbf [4/75] KVM: arm/arm64: consolidate arch timer trap handlers
> >> config: arm-axm55xx_defconfig (attached as .config)
> >> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> >> reproduce:
> >>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >>         chmod +x ~/bin/make.cross
> >>         git checkout 2b1265c58a873d917e99ac762e243c1274481dbf
> >>         # save the attached .config to linux build tree
> >>         GCC_VERSION=7.2.0 make.cross ARCH=arm 
> >>
> >> All errors (new ones prefixed by >>):
> > 
> > I was looking at this yesterday: It's a bit nasty, don't know a good
> > solution beside bringing back this part of my original timer rework series.
> > The problem is that those symbols contains the Aarch64 specific
> > (instruction) encoding of the timer registers, plus we need the AArch32
> > encodings for 32-on-64 guests.
> 
> Why? There is exactly one timer that needs trapping for AArch32 (the EL1
> physical timer). All we need is:
> 
> - the SYS_AARCH32_CNTP_* encodings on 32bit
> - some CPP magic to prevent the compilation from breaking
> 
> > 
> > That's why I used the generic UAPI encoding for the registers, because
> > we only need *some* identification for them, it doesn't need to be
> > something defined by the architecture.
> 
> I disagree. By doing so, you're conflating userspace access and
> trapping, which has proved to be a bad idea in the past. For example,
> you'd end-up having both CVAL and TVAL in UAPI, which is not something
> I'm keen to have. On the other hand, the trapping function do need to be
> able to handle these.
> 
I think it probably makes sense to have the sysreg encoding stuff in the
arch-specific files, and have an indirection in sys_regs.c and coproc.c
which 'translates' from a system register to some generic arch timer
define identifying a 'timer'.

I think the major breakage in the previous design was to use the same
*functions* for uaccess and for sysregs traps, but I don't think it was
necessarily a problem to use the same definitions to identify a timer.

That of couse leaves the problem of how to identify a register within a
timer.  Could we do something as braindead as:


diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index ce441dda412c..1d7bd452e5fd 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -32,6 +32,13 @@ enum kvm_arch_timers {
 	NR_KVM_TIMERS
 };
 
+enum arch_timer_reg {
+	ARCH_TIMER_REG_CTL,
+	ARCH_TIMER_REG_CVAL,
+	ARCH_TIMER_REG_TVAL,
+	ARCH_TIMER_REG_CNT
+}
+
 struct arch_timer_context {
 	struct kvm_vcpu			*vcpu;
 
@@ -86,8 +93,13 @@ bool kvm_timer_should_notify_user(struct kvm_vcpu *vcpu);
 void kvm_timer_update_run(struct kvm_vcpu *vcpu);
 void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu);
 
-u64 kvm_arm_timer_get_reg(struct kvm_vcpu *, u64 regid);
-int kvm_arm_timer_set_reg(struct kvm_vcpu *, u64 regid, u64 value);
+u64 kvm_arm_timer_get_reg(struct kvm_vcpu *,
+			  enum kvm_arch_timers timer,
+			  enum arch_timer_reg reg);
+int kvm_arm_timer_set_reg(struct kvm_vcpu *,
+			  enum kvm_arch_timers timer,
+			  enum arch_timer_reg reg,
+			  u64 value);
 
 u64 kvm_arm_timer_read_sysreg(struct kvm_vcpu *vcpu, u32 sr);
 void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu, u32 sr, u64 val);


Thanks,

    Christoffer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-09 10:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09  4:40 [kvmarm:kvm-arm64/nv-wip-v5.0-rc1 4/75] arch/arm/kvm/../../../virt/kvm/arm/arch_timer.c:700:7: error: 'SYS_CNTP_TVAL_EL0' undeclared kbuild test robot
2019-01-09  9:13 ` André Przywara
2019-01-09  9:13   ` André Przywara
2019-01-09 10:09   ` Marc Zyngier
2019-01-09 10:09     ` Marc Zyngier
2019-01-09 10:43     ` Andre Przywara
2019-01-09 10:43       ` Andre Przywara
2019-01-09 10:48     ` Christoffer Dall
2019-01-09 10:48       ` Christoffer Dall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.