| Message ID | 1490607072-21610-14-git-send-email-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Eric,
[auto build test ERROR on kvmarm/next]
[also build test ERROR on v4.11-rc4 next-20170327]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Eric-Auger/vITS-save-restore/20170327-195443
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c: In function 'vgic_its_has_attr':
>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: error: 'its' undeclared (first use in this function)
return vgic_its_table_restore(its);
^~~
arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: note: each undeclared identifier is reported only once for each function it appears in
vim +/its +1766 arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c
1750 case KVM_DEV_ARM_VGIC_CTRL_INIT:
1751 return 0;
1752 }
1753 break;
1754 case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: {
1755 u64 __user *uaddr = (u64 __user *)(long)attr->addr;
1756 u64 reg;
1757
1758 if (get_user(reg, uaddr))
1759 return -EFAULT;
1760
1761 return vgic_its_attr_regs_access(dev, attr, ®, true);
1762 }
1763 case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES:
1764 if (attr->attr)
1765 return -EINVAL;
> 1766 return vgic_its_table_restore(its);
1767 }
1768 return -ENXIO;
1769 }
1770
1771 static int vgic_its_set_attr(struct kvm_device *dev,
1772 struct kvm_device_attr *attr)
1773 {
1774 struct vgic_its *its = dev->private;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi, On 27/03/2017 17:04, kbuild test robot wrote: > Hi Eric, > > [auto build test ERROR on kvmarm/next] > [also build test ERROR on v4.11-rc4 next-20170327] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Eric-Auger/vITS-save-restore/20170327-195443 > base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next > config: arm64-defconfig (attached as .config) > compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 > reproduce: > wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=arm64 > > All errors (new ones prefixed by >>): > > arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c: In function 'vgic_its_has_attr': >>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: error: 'its' undeclared (first use in this function) > return vgic_its_table_restore(its); I think this is due to the fact the series is applied on KVM: arm/arm64: Emulate the EL1 phys timer registers (kvmarm-for-4.11). Please apply it to (kvmarm/queue, kvmarm/next, kvm_next) KVM: arm/arm64: vgic: Improve sync_hwstate performance (255905e) I also checked it compiles on 4.11-rc4. Please let me know if I got something wrong. Thanks Eric > ^~~ > arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: note: each undeclared identifier is reported only once for each function it appears in > > vim +/its +1766 arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c > > 1750 case KVM_DEV_ARM_VGIC_CTRL_INIT: > 1751 return 0; > 1752 } > 1753 break; > 1754 case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { > 1755 u64 __user *uaddr = (u64 __user *)(long)attr->addr; > 1756 u64 reg; > 1757 > 1758 if (get_user(reg, uaddr)) > 1759 return -EFAULT; > 1760 > 1761 return vgic_its_attr_regs_access(dev, attr, ®, true); > 1762 } > 1763 case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: > 1764 if (attr->attr) > 1765 return -EINVAL; >> 1766 return vgic_its_table_restore(its); > 1767 } > 1768 return -ENXIO; > 1769 } > 1770 > 1771 static int vgic_its_set_attr(struct kvm_device *dev, > 1772 struct kvm_device_attr *attr) > 1773 { > 1774 struct vgic_its *its = dev->private; > > --- > 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 >
Hi, Eric On 03/27, Auger Eric wrote: >Hi, > >On 27/03/2017 17:04, kbuild test robot wrote: >> Hi Eric, >> >> [auto build test ERROR on kvmarm/next] >> [also build test ERROR on v4.11-rc4 next-20170327] >> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] >> >> url: https://github.com/0day-ci/linux/commits/Eric-Auger/vITS-save-restore/20170327-195443 >> base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next >> config: arm64-defconfig (attached as .config) >> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 >> reproduce: >> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # save the attached .config to linux build tree >> make.cross ARCH=arm64 >> >> All errors (new ones prefixed by >>): >> >> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c: In function 'vgic_its_has_attr': >>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: error: 'its' undeclared (first use in this function) >> return vgic_its_table_restore(its); > >I think this is due to the fact the series is applied on KVM: arm/arm64: >Emulate the EL1 phys timer registers (kvmarm-for-4.11). > >Please apply it to (kvmarm/queue, kvmarm/next, kvm_next) KVM: arm/arm64: >vgic: Improve sync_hwstate performance (255905e) >I also checked it compiles on 4.11-rc4. Hmm, we did apply your series on top of kvarm/next, judging from the function code below, I can't see the 'its' declaration. static int vgic_its_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr) { switch (attr->group) { case KVM_DEV_ARM_VGIC_GRP_ADDR: switch (attr->attr) { case KVM_VGIC_ITS_ADDR_TYPE: return 0; } break; case KVM_DEV_ARM_VGIC_GRP_CTRL: switch (attr->attr) { case KVM_DEV_ARM_VGIC_CTRL_INIT: return 0; } break; case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { u64 __user *uaddr = (u64 __user *)(long)attr->addr; u64 reg; if (get_user(reg, uaddr)) return -EFAULT; return vgic_its_attr_regs_access(dev, attr, ®, true); } case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: if (attr->attr) return -EINVAL; return vgic_its_table_restore(its); } return -ENXIO; } Thanks, Xiaolong > >Please let me know if I got something wrong. > >Thanks > >Eric >> ^~~ >> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: note: each undeclared identifier is reported only once for each function it appears in >> >> vim +/its +1766 arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c >> >> 1750 case KVM_DEV_ARM_VGIC_CTRL_INIT: >> 1751 return 0; >> 1752 } >> 1753 break; >> 1754 case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { >> 1755 u64 __user *uaddr = (u64 __user *)(long)attr->addr; >> 1756 u64 reg; >> 1757 >> 1758 if (get_user(reg, uaddr)) >> 1759 return -EFAULT; >> 1760 >> 1761 return vgic_its_attr_regs_access(dev, attr, ®, true); >> 1762 } >> 1763 case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: >> 1764 if (attr->attr) >> 1765 return -EINVAL; >>> 1766 return vgic_its_table_restore(its); >> 1767 } >> 1768 return -ENXIO; >> 1769 } >> 1770 >> 1771 static int vgic_its_set_attr(struct kvm_device *dev, >> 1772 struct kvm_device_attr *attr) >> 1773 { >> 1774 struct vgic_its *its = dev->private; >> >> --- >> 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 >> >_______________________________________________ >kbuild-all mailing list >kbuild-all@lists.01.org >https://lists.01.org/mailman/listinfo/kbuild-all
Hi Xiaolong On 30/03/2017 04:21, Ye Xiaolong wrote: > Hi, Eric > > On 03/27, Auger Eric wrote: >> Hi, >> >> On 27/03/2017 17:04, kbuild test robot wrote: >>> Hi Eric, >>> >>> [auto build test ERROR on kvmarm/next] >>> [also build test ERROR on v4.11-rc4 next-20170327] >>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] >>> >>> url: https://github.com/0day-ci/linux/commits/Eric-Auger/vITS-save-restore/20170327-195443 >>> base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next >>> config: arm64-defconfig (attached as .config) >>> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 >>> reproduce: >>> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >>> chmod +x ~/bin/make.cross >>> # save the attached .config to linux build tree >>> make.cross ARCH=arm64 >>> >>> All errors (new ones prefixed by >>): >>> >>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c: In function 'vgic_its_has_attr': >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: error: 'its' undeclared (first use in this function) >>> return vgic_its_table_restore(its); >> >> I think this is due to the fact the series is applied on KVM: arm/arm64: >> Emulate the EL1 phys timer registers (kvmarm-for-4.11). >> >> Please apply it to (kvmarm/queue, kvmarm/next, kvm_next) KVM: arm/arm64: >> vgic: Improve sync_hwstate performance (255905e) >> I also checked it compiles on 4.11-rc4. > > Hmm, we did apply your series on top of kvarm/next, judging from the function > code below, I can't see the 'its' declaration. > > static int vgic_its_has_attr(struct kvm_device *dev, > struct kvm_device_attr *attr) > { > switch (attr->group) { > case KVM_DEV_ARM_VGIC_GRP_ADDR: > switch (attr->attr) { > case KVM_VGIC_ITS_ADDR_TYPE: > return 0; > } > break; > case KVM_DEV_ARM_VGIC_GRP_CTRL: > switch (attr->attr) { > case KVM_DEV_ARM_VGIC_CTRL_INIT: > return 0; > } > break; > case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { > u64 __user *uaddr = (u64 __user *)(long)attr->addr; > u64 reg; > > if (get_user(reg, uaddr)) > return -EFAULT; > > return vgic_its_attr_regs_access(dev, attr, ®, true); > } > case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: > if (attr->attr) > return -EINVAL; > return vgic_its_table_restore(its); The above code shows that my series is not correctly applied since that call to vgic_its_table_restore() should normally be in vgic_its_set_attr() and not in vgic_its_has_attr() function (virt/kvm/arm/vgic-its.c). I pushed this series applied on top of up to date kvmarm/next at https://github.com/eauger/linux/tree/v4.11-rc3-its-mig-v4. As noticed earlier, the series is applied on top of 255905e (KVM: arm/arm64: vgic: Improve sync_hwstate performance) Thanks Eric > } > return -ENXIO; > } > > Thanks, > Xiaolong > >> >> Please let me know if I got something wrong. >> >> Thanks >> >> Eric >>> ^~~ >>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: note: each undeclared identifier is reported only once for each function it appears in >>> >>> vim +/its +1766 arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c >>> >>> 1750 case KVM_DEV_ARM_VGIC_CTRL_INIT: >>> 1751 return 0; >>> 1752 } >>> 1753 break; >>> 1754 case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { >>> 1755 u64 __user *uaddr = (u64 __user *)(long)attr->addr; >>> 1756 u64 reg; >>> 1757 >>> 1758 if (get_user(reg, uaddr)) >>> 1759 return -EFAULT; >>> 1760 >>> 1761 return vgic_its_attr_regs_access(dev, attr, ®, true); >>> 1762 } >>> 1763 case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: >>> 1764 if (attr->attr) >>> 1765 return -EINVAL; >>>> 1766 return vgic_its_table_restore(its); >>> 1767 } >>> 1768 return -ENXIO; >>> 1769 } >>> 1770 >>> 1771 static int vgic_its_set_attr(struct kvm_device *dev, >>> 1772 struct kvm_device_attr *attr) >>> 1773 { >>> 1774 struct vgic_its *its = dev->private; >>> >>> --- >>> 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 >>> >> _______________________________________________ >> kbuild-all mailing list >> kbuild-all@lists.01.org >> https://lists.01.org/mailman/listinfo/kbuild-all > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 03/30, Auger Eric wrote: >Hi Xiaolong > >On 30/03/2017 04:21, Ye Xiaolong wrote: >> Hi, Eric >> >> On 03/27, Auger Eric wrote: >>> Hi, >>> >>> On 27/03/2017 17:04, kbuild test robot wrote: >>>> Hi Eric, >>>> >>>> [auto build test ERROR on kvmarm/next] >>>> [also build test ERROR on v4.11-rc4 next-20170327] >>>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] >>>> >>>> url: https://github.com/0day-ci/linux/commits/Eric-Auger/vITS-save-restore/20170327-195443 >>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next >>>> config: arm64-defconfig (attached as .config) >>>> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 >>>> reproduce: >>>> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >>>> chmod +x ~/bin/make.cross >>>> # save the attached .config to linux build tree >>>> make.cross ARCH=arm64 >>>> >>>> All errors (new ones prefixed by >>): >>>> >>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c: In function 'vgic_its_has_attr': >>>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: error: 'its' undeclared (first use in this function) >>>> return vgic_its_table_restore(its); >>> >>> I think this is due to the fact the series is applied on KVM: arm/arm64: >>> Emulate the EL1 phys timer registers (kvmarm-for-4.11). >>> >>> Please apply it to (kvmarm/queue, kvmarm/next, kvm_next) KVM: arm/arm64: >>> vgic: Improve sync_hwstate performance (255905e) >>> I also checked it compiles on 4.11-rc4. >> >> Hmm, we did apply your series on top of kvarm/next, judging from the function >> code below, I can't see the 'its' declaration. >> >> static int vgic_its_has_attr(struct kvm_device *dev, >> struct kvm_device_attr *attr) >> { >> switch (attr->group) { >> case KVM_DEV_ARM_VGIC_GRP_ADDR: >> switch (attr->attr) { >> case KVM_VGIC_ITS_ADDR_TYPE: >> return 0; >> } >> break; >> case KVM_DEV_ARM_VGIC_GRP_CTRL: >> switch (attr->attr) { >> case KVM_DEV_ARM_VGIC_CTRL_INIT: >> return 0; >> } >> break; >> case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { >> u64 __user *uaddr = (u64 __user *)(long)attr->addr; >> u64 reg; >> >> if (get_user(reg, uaddr)) >> return -EFAULT; >> >> return vgic_its_attr_regs_access(dev, attr, ®, true); >> } >> case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: >> if (attr->attr) >> return -EINVAL; >> return vgic_its_table_restore(its); >The above code shows that my series is not correctly applied since that >call to vgic_its_table_restore() should normally be in >vgic_its_set_attr() and not in vgic_its_has_attr() function >(virt/kvm/arm/vgic-its.c). > Er, you are right, the real problem here seems we had a rather old kvmarm/next mirror at the time 0day applied your series. Thanks for the clarification and sorry for the noise. Thanks, Xiaolong >I pushed this series applied on top of up to date kvmarm/next at >https://github.com/eauger/linux/tree/v4.11-rc3-its-mig-v4. > >As noticed earlier, the series is applied on top of 255905e (KVM: >arm/arm64: vgic: Improve sync_hwstate performance) > >Thanks > >Eric > > >> } >> return -ENXIO; >> } >> >> Thanks, >> Xiaolong >> >>> >>> Please let me know if I got something wrong. >>> >>> Thanks >>> >>> Eric >>>> ^~~ >>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: note: each undeclared identifier is reported only once for each function it appears in >>>> >>>> vim +/its +1766 arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c >>>> >>>> 1750 case KVM_DEV_ARM_VGIC_CTRL_INIT: >>>> 1751 return 0; >>>> 1752 } >>>> 1753 break; >>>> 1754 case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { >>>> 1755 u64 __user *uaddr = (u64 __user *)(long)attr->addr; >>>> 1756 u64 reg; >>>> 1757 >>>> 1758 if (get_user(reg, uaddr)) >>>> 1759 return -EFAULT; >>>> 1760 >>>> 1761 return vgic_its_attr_regs_access(dev, attr, ®, true); >>>> 1762 } >>>> 1763 case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: >>>> 1764 if (attr->attr) >>>> 1765 return -EINVAL; >>>>> 1766 return vgic_its_table_restore(its); >>>> 1767 } >>>> 1768 return -ENXIO; >>>> 1769 } >>>> 1770 >>>> 1771 static int vgic_its_set_attr(struct kvm_device *dev, >>>> 1772 struct kvm_device_attr *attr) >>>> 1773 { >>>> 1774 struct vgic_its *its = dev->private; >>>> >>>> --- >>>> 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 >>>> >>> _______________________________________________ >>> kbuild-all mailing list >>> kbuild-all@lists.01.org >>> https://lists.01.org/mailman/listinfo/kbuild-all >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>
Hi Xiaolong, On 30/03/2017 09:29, Ye Xiaolong wrote: > On 03/30, Auger Eric wrote: >> Hi Xiaolong >> >> On 30/03/2017 04:21, Ye Xiaolong wrote: >>> Hi, Eric >>> >>> On 03/27, Auger Eric wrote: >>>> Hi, >>>> >>>> On 27/03/2017 17:04, kbuild test robot wrote: >>>>> Hi Eric, >>>>> >>>>> [auto build test ERROR on kvmarm/next] >>>>> [also build test ERROR on v4.11-rc4 next-20170327] >>>>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] >>>>> >>>>> url: https://github.com/0day-ci/linux/commits/Eric-Auger/vITS-save-restore/20170327-195443 >>>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next >>>>> config: arm64-defconfig (attached as .config) >>>>> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 >>>>> reproduce: >>>>> wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >>>>> chmod +x ~/bin/make.cross >>>>> # save the attached .config to linux build tree >>>>> make.cross ARCH=arm64 >>>>> >>>>> All errors (new ones prefixed by >>): >>>>> >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c: In function 'vgic_its_has_attr': >>>>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: error: 'its' undeclared (first use in this function) >>>>> return vgic_its_table_restore(its); >>>> >>>> I think this is due to the fact the series is applied on KVM: arm/arm64: >>>> Emulate the EL1 phys timer registers (kvmarm-for-4.11). >>>> >>>> Please apply it to (kvmarm/queue, kvmarm/next, kvm_next) KVM: arm/arm64: >>>> vgic: Improve sync_hwstate performance (255905e) >>>> I also checked it compiles on 4.11-rc4. >>> >>> Hmm, we did apply your series on top of kvarm/next, judging from the function >>> code below, I can't see the 'its' declaration. >>> >>> static int vgic_its_has_attr(struct kvm_device *dev, >>> struct kvm_device_attr *attr) >>> { >>> switch (attr->group) { >>> case KVM_DEV_ARM_VGIC_GRP_ADDR: >>> switch (attr->attr) { >>> case KVM_VGIC_ITS_ADDR_TYPE: >>> return 0; >>> } >>> break; >>> case KVM_DEV_ARM_VGIC_GRP_CTRL: >>> switch (attr->attr) { >>> case KVM_DEV_ARM_VGIC_CTRL_INIT: >>> return 0; >>> } >>> break; >>> case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { >>> u64 __user *uaddr = (u64 __user *)(long)attr->addr; >>> u64 reg; >>> >>> if (get_user(reg, uaddr)) >>> return -EFAULT; >>> >>> return vgic_its_attr_regs_access(dev, attr, ®, true); >>> } >>> case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: >>> if (attr->attr) >>> return -EINVAL; >>> return vgic_its_table_restore(its); >> The above code shows that my series is not correctly applied since that >> call to vgic_its_table_restore() should normally be in >> vgic_its_set_attr() and not in vgic_its_has_attr() function >> (virt/kvm/arm/vgic-its.c). >> > > Er, you are right, the real problem here seems we had a rather old kvmarm/next mirror > at the time 0day applied your series. > > Thanks for the clarification and sorry for the noise. no worries Thanks Eric > > Thanks, > Xiaolong >> I pushed this series applied on top of up to date kvmarm/next at >> https://github.com/eauger/linux/tree/v4.11-rc3-its-mig-v4. >> >> As noticed earlier, the series is applied on top of 255905e (KVM: >> arm/arm64: vgic: Improve sync_hwstate performance) >> >> Thanks >> >> Eric >> >> >>> } >>> return -ENXIO; >>> } >>> >>> Thanks, >>> Xiaolong >>> >>>> >>>> Please let me know if I got something wrong. >>>> >>>> Thanks >>>> >>>> Eric >>>>> ^~~ >>>>> arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c:1766:33: note: each undeclared identifier is reported only once for each function it appears in >>>>> >>>>> vim +/its +1766 arch/arm64/kvm/../../../virt/kvm/arm/vgic/vgic-its.c >>>>> >>>>> 1750 case KVM_DEV_ARM_VGIC_CTRL_INIT: >>>>> 1751 return 0; >>>>> 1752 } >>>>> 1753 break; >>>>> 1754 case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: { >>>>> 1755 u64 __user *uaddr = (u64 __user *)(long)attr->addr; >>>>> 1756 u64 reg; >>>>> 1757 >>>>> 1758 if (get_user(reg, uaddr)) >>>>> 1759 return -EFAULT; >>>>> 1760 >>>>> 1761 return vgic_its_attr_regs_access(dev, attr, ®, true); >>>>> 1762 } >>>>> 1763 case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: >>>>> 1764 if (attr->attr) >>>>> 1765 return -EINVAL; >>>>>> 1766 return vgic_its_table_restore(its); >>>>> 1767 } >>>>> 1768 return -ENXIO; >>>>> 1769 } >>>>> 1770 >>>>> 1771 static int vgic_its_set_attr(struct kvm_device *dev, >>>>> 1772 struct kvm_device_attr *attr) >>>>> 1773 { >>>>> 1774 struct vgic_its *its = dev->private; >>>>> >>>>> --- >>>>> 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 >>>>> >>>> _______________________________________________ >>>> kbuild-all mailing list >>>> kbuild-all@lists.01.org >>>> https://lists.01.org/mailman/listinfo/kbuild-all >>> >>> _______________________________________________ >>> linux-arm-kernel mailing list >>> linux-arm-kernel@lists.infradead.org >>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >>>
On Mon, Mar 27 2017 at 10:31:03 AM, Eric Auger <eric.auger@redhat.com> wrote: > Introduce a new group aiming at saving/restoring the ITS > tables to/from the guest memory. > > We hold the vcpus lock during the save and restore to make > sure no vcpu is running. > > At this stage the functionality is not yet implemented. Only > the skeleton is put in place. > > The ABI revision supposed to have been set through IIDR user > write is checked before the table restoration. This guarantees > this vITS knows how to restore the saved tables. This last point hints at the kernel side being able to deal with multiple versions of the ABI. As I mentioned before, this requires some clarification on what we plan to support, and whether or not we are able to deprecate ABIs in the long run One thing that is not clear to me is that although you want to be able to restore the vITS using a given ABI, should you be able to save it using that same ABI? Or does a restore/save cycle act as an ABI upgrade for this VM? I'd also like to see some infrastructure in the code to support this, in the form of a per-ABI array of support functions (even if we only have one ABI for the time being). Although that's not required ATM, it would certainly halp me understanding what is ABI-specific and what's not. Thanks, M.
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h index 4beb83b..7b165e9 100644 --- a/arch/arm/include/uapi/asm/kvm.h +++ b/arch/arm/include/uapi/asm/kvm.h @@ -193,6 +193,7 @@ struct kvm_arch_memory_slot { #define KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS 6 #define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7 #define KVM_DEV_ARM_VGIC_GRP_ITS_REGS 8 +#define KVM_DEV_ARM_VGIC_GRP_ITS_TABLES 9 #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 10 #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \ (0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT) diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 7e8dd69..166df68 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -213,6 +213,7 @@ struct kvm_arch_memory_slot { #define KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS 6 #define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7 #define KVM_DEV_ARM_VGIC_GRP_ITS_REGS 8 +#define KVM_DEV_ARM_VGIC_GRP_ITS_TABLES 9 #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 10 #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \ (0x3fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT) diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c index b275aea..76dd562 100644 --- a/virt/kvm/arm/vgic/vgic-its.c +++ b/virt/kvm/arm/vgic/vgic-its.c @@ -1627,6 +1627,135 @@ int vgic_its_attr_regs_access(struct kvm_device *dev, return ret; } +/** + * vgic_its_flush_pending_tables - Flush the pending tables into guest RAM + */ +static int vgic_its_flush_pending_tables(struct kvm *kvm) +{ + return -ENXIO; +} + +/** + * vgic_its_restore_pending_tables - Restore the pending tables from guest + * RAM to internal data structs + */ +static int vgic_its_restore_pending_tables(struct kvm *kvm) +{ + return -ENXIO; +} + +/** + * vgic_its_flush_device_tables - flush the device table and all ITT + * into guest RAM + */ +static int vgic_its_flush_device_tables(struct vgic_its *its) +{ + return -ENXIO; +} + +/** + * vgic_its_restore_device_tables - restore the device table and all ITT + * from guest RAM to internal data structs + */ +static int vgic_its_restore_device_tables(struct vgic_its *its) +{ + return -ENXIO; +} + +/** + * vgic_its_flush_collection_table - flush the collection table into + * guest RAM + */ +static int vgic_its_flush_collection_table(struct vgic_its *its) +{ + return -ENXIO; +} + +/** + * vgic_its_restore_collection_table - reads the collection table + * in guest memory and restores the ITS internal state. Requires the + * BASER registers to be restored before. + */ +static int vgic_its_restore_collection_table(struct vgic_its *its) +{ + return -ENXIO; +} + +/** + * vgic_its_table_flush - Flush all the tables into guest RAM + */ +static int vgic_its_table_flush(struct vgic_its *its) +{ + struct kvm *kvm = its->dev->kvm; + int ret; + + mutex_lock(&kvm->lock); + + if (!lock_all_vcpus(kvm)) { + mutex_unlock(&kvm->lock); + return -EBUSY; + } + + ret = vgic_its_flush_pending_tables(kvm); + if (ret) + goto out; + ret = vgic_its_flush_device_tables(its); + if (ret) + goto out; + + ret = vgic_its_flush_collection_table(its); + +out: + unlock_all_vcpus(kvm); + mutex_unlock(&kvm->lock); + return ret; +} + +/** + * vgic_its_table_restore - Restore all tables from guest RAM to internal + * data structs + */ +static int vgic_its_table_restore(struct vgic_its *its) +{ + struct kvm *kvm = its->dev->kvm; + int ret; + + mutex_lock(&kvm->lock); + + if (its->user_revision < REV) { + mutex_unlock(&kvm->lock); + return -EINVAL; + } + + if (!lock_all_vcpus(kvm)) { + mutex_unlock(&kvm->lock); + return -EBUSY; + } + + ret = vgic_its_restore_collection_table(its); + if (ret) + goto out; + + ret = vgic_its_restore_device_tables(its); + if (ret) + goto out; + + ret = vgic_its_restore_pending_tables(kvm); + +out: + unlock_all_vcpus(kvm); + mutex_unlock(&kvm->lock); + + if (ret) + return ret; + + /* + * On restore path, MSI injections can happen before the + * first VCPU run so let's complete the GIC init here. + */ + return kvm_vgic_map_resources(its->dev->kvm); +} + static int vgic_its_has_attr(struct kvm_device *dev, struct kvm_device_attr *attr) { @@ -1645,6 +1774,8 @@ static int vgic_its_has_attr(struct kvm_device *dev, break; case KVM_DEV_ARM_VGIC_GRP_ITS_REGS: return vgic_its_has_attr_regs(dev, attr); + case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: + return 0; } return -ENXIO; } @@ -1693,6 +1824,10 @@ static int vgic_its_set_attr(struct kvm_device *dev, return vgic_its_attr_regs_access(dev, attr, ®, true); } + case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: + if (attr->attr) + return -EINVAL; + return vgic_its_table_restore(its); } return -ENXIO; } @@ -1700,9 +1835,10 @@ static int vgic_its_set_attr(struct kvm_device *dev, static int vgic_its_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr) { + struct vgic_its *its = dev->private; + switch (attr->group) { case KVM_DEV_ARM_VGIC_GRP_ADDR: { - struct vgic_its *its = dev->private; u64 addr = its->vgic_its_base; u64 __user *uaddr = (u64 __user *)(long)attr->addr; unsigned long type = (unsigned long)attr->attr; @@ -1723,6 +1859,10 @@ static int vgic_its_get_attr(struct kvm_device *dev, if (ret) return ret; return put_user(reg, uaddr); + case KVM_DEV_ARM_VGIC_GRP_ITS_TABLES: + if (attr->attr) + return -EINVAL; + return vgic_its_table_flush(its); } default: return -ENXIO;
Introduce a new group aiming at saving/restoring the ITS tables to/from the guest memory. We hold the vcpus lock during the save and restore to make sure no vcpu is running. At this stage the functionality is not yet implemented. Only the skeleton is put in place. The ABI revision supposed to have been set through IIDR user write is checked before the table restoration. This guarantees this vITS knows how to restore the saved tables. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- v3 -> v4: - pass kvm struct handle to vgic_its_flush/restore_pending_tables - take the kvm lock and vcpu locks - ABI revision check - check attr->attr is null v1 -> v2: - remove useless kvm parameter --- arch/arm/include/uapi/asm/kvm.h | 1 + arch/arm64/include/uapi/asm/kvm.h | 1 + virt/kvm/arm/vgic/vgic-its.c | 142 +++++++++++++++++++++++++++++++++++++- 3 files changed, 143 insertions(+), 1 deletion(-)