docs/man/xl.cfg.5.pod.in | 57 +++++++++ tools/libxl/libxl.h | 16 +++ tools/libxl/libxl_create.c | 30 ++++- tools/libxl/libxl_mem.c | 6 +- tools/libxl/libxl_types.idl | 9 ++ tools/libxl/libxl_utils.c | 15 +++ tools/libxl/libxl_utils.h | 1 + tools/ocaml/libs/xc/abi-check | 17 +-- tools/ocaml/libs/xc/xenctrl.ml | 4 + tools/ocaml/libs/xc/xenctrl.mli | 5 + tools/ocaml/libs/xc/xenctrl_stubs.c | 17 ++- tools/xl/xl_parse.c | 178 ++++++++++++++++++-------- xen/arch/arm/Kconfig | 1 + xen/arch/arm/domain.c | 10 +- xen/arch/arm/p2m.c | 2 +- xen/arch/x86/dom0_build.c | 2 +- xen/arch/x86/domain.c | 2 +- xen/arch/x86/hvm/mtrr.c | 5 +- xen/arch/x86/mm/mem_sharing.c | 2 +- xen/arch/x86/mm/p2m.c | 4 +- xen/arch/x86/mm/paging.c | 2 +- xen/arch/x86/x86_64/mm.c | 2 +- xen/common/domain.c | 7 + xen/common/domctl.c | 13 -- xen/common/memory.c | 4 +- xen/common/vm_event.c | 2 +- xen/drivers/passthrough/Kconfig | 3 + xen/drivers/passthrough/device_tree.c | 11 -- xen/drivers/passthrough/iommu.c | 147 ++++++--------------- xen/drivers/passthrough/pci.c | 12 -- xen/drivers/passthrough/vtd/iommu.c | 10 +- xen/drivers/passthrough/x86/iommu.c | 97 -------------- xen/include/asm-arm/iommu.h | 3 - xen/include/asm-x86/iommu.h | 4 - xen/include/public/domctl.h | 10 +- xen/include/xen/iommu.h | 40 +++--- xen/include/xen/sched.h | 8 -- 37 files changed, 388 insertions(+), 370 deletions(-)
These are the remaining uncommitted patches from my previous series: https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg01208.html The only patch that has been revised is patch #4 (previously patch #6). Ian Jackson (1): tools/ocaml: abi check: Cope with consecutive relevant enums Paul Durrant (3): remove late (on-demand) construction of IOMMU page tables iommu: tidy up iommu_use_hap_pt() and need_iommu_pt_sync() macros introduce a 'passthrough' configuration option to xl.cfg... docs/man/xl.cfg.5.pod.in | 57 +++++++++ tools/libxl/libxl.h | 16 +++ tools/libxl/libxl_create.c | 30 ++++- tools/libxl/libxl_mem.c | 6 +- tools/libxl/libxl_types.idl | 9 ++ tools/libxl/libxl_utils.c | 15 +++ tools/libxl/libxl_utils.h | 1 + tools/ocaml/libs/xc/abi-check | 17 +-- tools/ocaml/libs/xc/xenctrl.ml | 4 + tools/ocaml/libs/xc/xenctrl.mli | 5 + tools/ocaml/libs/xc/xenctrl_stubs.c | 17 ++- tools/xl/xl_parse.c | 178 ++++++++++++++++++-------- xen/arch/arm/Kconfig | 1 + xen/arch/arm/domain.c | 10 +- xen/arch/arm/p2m.c | 2 +- xen/arch/x86/dom0_build.c | 2 +- xen/arch/x86/domain.c | 2 +- xen/arch/x86/hvm/mtrr.c | 5 +- xen/arch/x86/mm/mem_sharing.c | 2 +- xen/arch/x86/mm/p2m.c | 4 +- xen/arch/x86/mm/paging.c | 2 +- xen/arch/x86/x86_64/mm.c | 2 +- xen/common/domain.c | 7 + xen/common/domctl.c | 13 -- xen/common/memory.c | 4 +- xen/common/vm_event.c | 2 +- xen/drivers/passthrough/Kconfig | 3 + xen/drivers/passthrough/device_tree.c | 11 -- xen/drivers/passthrough/iommu.c | 147 ++++++--------------- xen/drivers/passthrough/pci.c | 12 -- xen/drivers/passthrough/vtd/iommu.c | 10 +- xen/drivers/passthrough/x86/iommu.c | 97 -------------- xen/include/asm-arm/iommu.h | 3 - xen/include/asm-x86/iommu.h | 4 - xen/include/public/domctl.h | 10 +- xen/include/xen/iommu.h | 40 +++--- xen/include/xen/sched.h | 8 -- 37 files changed, 388 insertions(+), 370 deletions(-) --- Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Anthony PERARD <anthony.perard@citrix.com> Cc: David Scott <dave@recoil.org> Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Petre Pircalabu <ppircalabu@bitdefender.com> Cc: "Roger Pau Monné" <roger.pau@citrix.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tamas K Lengyel <tamas@tklengyel.com> Cc: Tim Deegan <tim@xen.org> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> Cc: Wei Liu <wl@xen.org> -- 2.20.1.2.gb21ebb671 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Ping? I don't think this series is awaiting any more acks. Paul > -----Original Message----- > From: Paul Durrant <paul.durrant@citrix.com> > Sent: 18 September 2019 11:41 > To: xen-devel@lists.xenproject.org > Cc: Paul Durrant <Paul.Durrant@citrix.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Anthony Perard > <anthony.perard@citrix.com>; David Scott <dave@recoil.org>; George Dunlap <George.Dunlap@citrix.com>; > Ian Jackson <Ian.Jackson@citrix.com>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Petre Pircalabu > <ppircalabu@bitdefender.com>; Roger Pau Monne <roger.pau@citrix.com>; Stefano Stabellini > <sstabellini@kernel.org>; Tamas K Lengyel <tamas@tklengyel.com>; Tim (Xen.org) <tim@xen.org>; > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Wei Liu <wl@xen.org> > Subject: [PATCH v13 0/4] add per-domain IOMMU control > > These are the remaining uncommitted patches from my previous series: > > https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg01208.html > > The only patch that has been revised is patch #4 (previously patch #6). > > Ian Jackson (1): > tools/ocaml: abi check: Cope with consecutive relevant enums > > Paul Durrant (3): > remove late (on-demand) construction of IOMMU page tables > iommu: tidy up iommu_use_hap_pt() and need_iommu_pt_sync() macros > introduce a 'passthrough' configuration option to xl.cfg... > > docs/man/xl.cfg.5.pod.in | 57 +++++++++ > tools/libxl/libxl.h | 16 +++ > tools/libxl/libxl_create.c | 30 ++++- > tools/libxl/libxl_mem.c | 6 +- > tools/libxl/libxl_types.idl | 9 ++ > tools/libxl/libxl_utils.c | 15 +++ > tools/libxl/libxl_utils.h | 1 + > tools/ocaml/libs/xc/abi-check | 17 +-- > tools/ocaml/libs/xc/xenctrl.ml | 4 + > tools/ocaml/libs/xc/xenctrl.mli | 5 + > tools/ocaml/libs/xc/xenctrl_stubs.c | 17 ++- > tools/xl/xl_parse.c | 178 ++++++++++++++++++-------- > xen/arch/arm/Kconfig | 1 + > xen/arch/arm/domain.c | 10 +- > xen/arch/arm/p2m.c | 2 +- > xen/arch/x86/dom0_build.c | 2 +- > xen/arch/x86/domain.c | 2 +- > xen/arch/x86/hvm/mtrr.c | 5 +- > xen/arch/x86/mm/mem_sharing.c | 2 +- > xen/arch/x86/mm/p2m.c | 4 +- > xen/arch/x86/mm/paging.c | 2 +- > xen/arch/x86/x86_64/mm.c | 2 +- > xen/common/domain.c | 7 + > xen/common/domctl.c | 13 -- > xen/common/memory.c | 4 +- > xen/common/vm_event.c | 2 +- > xen/drivers/passthrough/Kconfig | 3 + > xen/drivers/passthrough/device_tree.c | 11 -- > xen/drivers/passthrough/iommu.c | 147 ++++++--------------- > xen/drivers/passthrough/pci.c | 12 -- > xen/drivers/passthrough/vtd/iommu.c | 10 +- > xen/drivers/passthrough/x86/iommu.c | 97 -------------- > xen/include/asm-arm/iommu.h | 3 - > xen/include/asm-x86/iommu.h | 4 - > xen/include/public/domctl.h | 10 +- > xen/include/xen/iommu.h | 40 +++--- > xen/include/xen/sched.h | 8 -- > 37 files changed, 388 insertions(+), 370 deletions(-) > --- > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > Cc: Anthony PERARD <anthony.perard@citrix.com> > Cc: David Scott <dave@recoil.org> > Cc: George Dunlap <george.dunlap@eu.citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Petre Pircalabu <ppircalabu@bitdefender.com> > Cc: "Roger Pau Monné" <roger.pau@citrix.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Tamas K Lengyel <tamas@tklengyel.com> > Cc: Tim Deegan <tim@xen.org> > Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> > Cc: Wei Liu <wl@xen.org> > -- > 2.20.1.2.gb21ebb671 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 25.09.2019 10:40, Paul Durrant wrote: > Ping? I don't think this series is awaiting any more acks. I did enumerate the other day on irc what is still missing according to my records. Would you mind pointing me at a libxl maintainer ack for patch 1? I think I've managed to spot the missing one for patch 3, which hung off a "REPOST" thread. And I realize I was wrongly looking for a tool stack maintainer ack for patch 4, when a libxl one is sufficient and already there. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: 25 September 2019 09:51 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: xen-devel@lists.xenproject.org; Petre Pircalabu <ppircalabu@bitdefender.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; George Dunlap > <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Stefano Stabellini > <sstabellini@kernel.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; David Scott > <dave@recoil.org>; Tamas K Lengyel <tamas@tklengyel.com>; Tim (Xen.org) <tim@xen.org>; Wei Liu > <wl@xen.org> > Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control > > On 25.09.2019 10:40, Paul Durrant wrote: > > Ping? I don't think this series is awaiting any more acks. > > I did enumerate the other day on irc what is still missing according > to my records. Oh, I must have dropped off an missed that. Connectivity can be somewhat flakey here in the office :-( > Would you mind pointing me at a libxl maintainer ack > for patch 1? I think I've managed to spot the missing one for patch 3, > which hung off a "REPOST" thread. And I realize I was wrongly looking > for a tool stack maintainer ack for patch 4, when a libxl one is > sufficient and already there. I thought I'd had an toolstack ack on patch #1 but now that I look again, you're right... it's still missing. I'll chase that one. Thanks, Paul > > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[CC Julien] Hi Paul I may mistake, but looks like 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up iommu_use_hap_pt() and need_iommu_pt_sync() macros triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) . So, iommu_setup() calls clear_iommu_hap_pt_share() with iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which, actually, triggers ASSERT. ... (XEN) Assertion 'unreachable' failed at ...ild-workspace/build/xen/xen/include/xen/iommu.h:72 (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]---- (XEN) CPU: 0 (XEN) PC: 00000000002b3ae0 iommu_setup+0xa0/0x18c (XEN) LR: 00000000002b3a8c (XEN) SP: 00000000002f7dc0 (XEN) CPSR: a0000249 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 00000000002a7000 X1: 0000000000000000 X2: 2c736173656e6572 (XEN) X3: 0000000000000002 X4: 0000000000000001 X5: 0000000000000000 (XEN) X6: 0000000000000080 X7: 2b726072646d6471 X8: 7f7f7f7f7f7f7f7f (XEN) X9: ff65685e6c6f7275 X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101 (XEN) X12: 0000000000000038 X13: 0000000000280910 X14: 0000000000000020 (XEN) X15: 0000000000000000 X16: 00000000002a7000 X17: 00000000002a7000 (XEN) X18: 00000000002a7000 X19: 0000000000000000 X20: 00000000ffffffed (XEN) X21: 00000000002a6380 X22: 0000000000335430 X23: 0000000000000002 (XEN) X24: 000000000029b1f0 X25: 00000000002d83d0 X26: 0000000048000000 (XEN) X27: 00000000c0000000 X28: 0000000000000001 FP: 00000000002f7dc0 (XEN) (XEN) VTCR_EL2: 80000000 (XEN) VTTBR_EL2: 0000000000000000 (XEN) (XEN) SCTLR_EL2: 30cd183d (XEN) HCR_EL2: 0000000000000038 (XEN) TTBR0_EL2: 00000000781b4000 (XEN) (XEN) ESR_EL2: f2000001 (XEN) HPFAR_EL2: 0000000000000000 (XEN) FAR_EL2: 0000000000000000 (XEN) (XEN) Xen stack trace from sp=00000000002f7dc0: (XEN) 00000000002f7de0 00000000002bdd94 0000000000000002 0000000000000002 (XEN) 00000000bfe0b660 00000000002001b4 0000000078080000 0000000077e80000 (XEN) 0000000048000000 0000000000000000 0000000000400000 0000000000000003 (XEN) 0000000000000001 0000000000000000 0000000078080000 0000000048080040 (XEN) 0000000000000000 000000000000f080 0000000048000000 0000000078000000 (XEN) 00000000002d83c0 00000000002aa440 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000300000000 0000000000000000 00000040ffffffff (XEN) 0000000000000400 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) Xen call trace: (XEN) [<00000000002b3ae0>] iommu_setup+0xa0/0x18c (PC) (XEN) [<00000000002b3a8c>] iommu_setup+0x4c/0x18c (LR) (XEN) [<00000000002bdd94>] start_xen+0xaa0/0xc7c (XEN) [<00000000002001b4>] arm64/head.o#primary_switched+0xc/0x2c (XEN) (XEN) (XEN) **************************************** (XEN) Panic on CPU 0: (XEN) Assertion 'unreachable' failed at ...ild-workspace/build/xen/xen/include/xen/iommu.h:72 (XEN) **************************************** (XEN) -- Regards, Oleksandr Tyshchenko _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message----- > From: Oleksandr <olekstysh@gmail.com> > Sent: 25 September 2019 16:50 > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com> > Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu > <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George Dunlap > <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson > <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org; > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien Grall > <julien.grall@arm.com> > Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control > > > [CC Julien] > > > Hi Paul > > I may mistake, but looks like > > 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up > iommu_use_hap_pt() and need_iommu_pt_sync() macros > > triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built > with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) . Oh, I'm sure I was told this was not a possibility for ARM, which is why iommu_hap_pt_share ended up being #defined. Seems I was misled, in which case ARM ought to be have a more dynamic config. as with x86. Paul > > So, iommu_setup() calls clear_iommu_hap_pt_share() with > iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which, > actually, triggers ASSERT. > > ... > > > (XEN) Assertion 'unreachable' failed at > ...ild-workspace/build/xen/xen/include/xen/iommu.h:72 > (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]---- > (XEN) CPU: 0 > (XEN) PC: 00000000002b3ae0 iommu_setup+0xa0/0x18c > (XEN) LR: 00000000002b3a8c > (XEN) SP: 00000000002f7dc0 > (XEN) CPSR: a0000249 MODE:64-bit EL2h (Hypervisor, handler) > (XEN) X0: 00000000002a7000 X1: 0000000000000000 X2: 2c736173656e6572 > (XEN) X3: 0000000000000002 X4: 0000000000000001 X5: 0000000000000000 > (XEN) X6: 0000000000000080 X7: 2b726072646d6471 X8: 7f7f7f7f7f7f7f7f > (XEN) X9: ff65685e6c6f7275 X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101 > (XEN) X12: 0000000000000038 X13: 0000000000280910 X14: 0000000000000020 > (XEN) X15: 0000000000000000 X16: 00000000002a7000 X17: 00000000002a7000 > (XEN) X18: 00000000002a7000 X19: 0000000000000000 X20: 00000000ffffffed > (XEN) X21: 00000000002a6380 X22: 0000000000335430 X23: 0000000000000002 > (XEN) X24: 000000000029b1f0 X25: 00000000002d83d0 X26: 0000000048000000 > (XEN) X27: 00000000c0000000 X28: 0000000000000001 FP: 00000000002f7dc0 > (XEN) > (XEN) VTCR_EL2: 80000000 > (XEN) VTTBR_EL2: 0000000000000000 > (XEN) > (XEN) SCTLR_EL2: 30cd183d > (XEN) HCR_EL2: 0000000000000038 > (XEN) TTBR0_EL2: 00000000781b4000 > (XEN) > (XEN) ESR_EL2: f2000001 > (XEN) HPFAR_EL2: 0000000000000000 > (XEN) FAR_EL2: 0000000000000000 > (XEN) > (XEN) Xen stack trace from sp=00000000002f7dc0: > (XEN) 00000000002f7de0 00000000002bdd94 0000000000000002 0000000000000002 > (XEN) 00000000bfe0b660 00000000002001b4 0000000078080000 0000000077e80000 > (XEN) 0000000048000000 0000000000000000 0000000000400000 0000000000000003 > (XEN) 0000000000000001 0000000000000000 0000000078080000 0000000048080040 > (XEN) 0000000000000000 000000000000f080 0000000048000000 0000000078000000 > (XEN) 00000000002d83c0 00000000002aa440 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000300000000 0000000000000000 00000040ffffffff > (XEN) 0000000000000400 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) Xen call trace: > (XEN) [<00000000002b3ae0>] iommu_setup+0xa0/0x18c (PC) > (XEN) [<00000000002b3a8c>] iommu_setup+0x4c/0x18c (LR) > (XEN) [<00000000002bdd94>] start_xen+0xaa0/0xc7c > (XEN) [<00000000002001b4>] arm64/head.o#primary_switched+0xc/0x2c > (XEN) > (XEN) > (XEN) **************************************** > (XEN) Panic on CPU 0: > (XEN) Assertion 'unreachable' failed at > ...ild-workspace/build/xen/xen/include/xen/iommu.h:72 > (XEN) **************************************** > (XEN) > > > -- > Regards, > > Oleksandr Tyshchenko _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message----- > From: Oleksandr <olekstysh@gmail.com> > Sent: 25 September 2019 16:50 > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com> > Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu > <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George Dunlap > <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson > <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org; > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien Grall > <julien.grall@arm.com> > Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control > > > [CC Julien] > > > Hi Paul > > I may mistake, but looks like > > 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up > iommu_use_hap_pt() and need_iommu_pt_sync() macros > > triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built > with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) . > > So, iommu_setup() calls clear_iommu_hap_pt_share() with > iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which, > actually, triggers ASSERT. BTW, I assume disabling the iommu on the xen command like will work round the issue? Paul > > ... > > > (XEN) Assertion 'unreachable' failed at > ...ild-workspace/build/xen/xen/include/xen/iommu.h:72 > (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]---- > (XEN) CPU: 0 > (XEN) PC: 00000000002b3ae0 iommu_setup+0xa0/0x18c > (XEN) LR: 00000000002b3a8c > (XEN) SP: 00000000002f7dc0 > (XEN) CPSR: a0000249 MODE:64-bit EL2h (Hypervisor, handler) > (XEN) X0: 00000000002a7000 X1: 0000000000000000 X2: 2c736173656e6572 > (XEN) X3: 0000000000000002 X4: 0000000000000001 X5: 0000000000000000 > (XEN) X6: 0000000000000080 X7: 2b726072646d6471 X8: 7f7f7f7f7f7f7f7f > (XEN) X9: ff65685e6c6f7275 X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101 > (XEN) X12: 0000000000000038 X13: 0000000000280910 X14: 0000000000000020 > (XEN) X15: 0000000000000000 X16: 00000000002a7000 X17: 00000000002a7000 > (XEN) X18: 00000000002a7000 X19: 0000000000000000 X20: 00000000ffffffed > (XEN) X21: 00000000002a6380 X22: 0000000000335430 X23: 0000000000000002 > (XEN) X24: 000000000029b1f0 X25: 00000000002d83d0 X26: 0000000048000000 > (XEN) X27: 00000000c0000000 X28: 0000000000000001 FP: 00000000002f7dc0 > (XEN) > (XEN) VTCR_EL2: 80000000 > (XEN) VTTBR_EL2: 0000000000000000 > (XEN) > (XEN) SCTLR_EL2: 30cd183d > (XEN) HCR_EL2: 0000000000000038 > (XEN) TTBR0_EL2: 00000000781b4000 > (XEN) > (XEN) ESR_EL2: f2000001 > (XEN) HPFAR_EL2: 0000000000000000 > (XEN) FAR_EL2: 0000000000000000 > (XEN) > (XEN) Xen stack trace from sp=00000000002f7dc0: > (XEN) 00000000002f7de0 00000000002bdd94 0000000000000002 0000000000000002 > (XEN) 00000000bfe0b660 00000000002001b4 0000000078080000 0000000077e80000 > (XEN) 0000000048000000 0000000000000000 0000000000400000 0000000000000003 > (XEN) 0000000000000001 0000000000000000 0000000078080000 0000000048080040 > (XEN) 0000000000000000 000000000000f080 0000000048000000 0000000078000000 > (XEN) 00000000002d83c0 00000000002aa440 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000300000000 0000000000000000 00000040ffffffff > (XEN) 0000000000000400 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) Xen call trace: > (XEN) [<00000000002b3ae0>] iommu_setup+0xa0/0x18c (PC) > (XEN) [<00000000002b3a8c>] iommu_setup+0x4c/0x18c (LR) > (XEN) [<00000000002bdd94>] start_xen+0xaa0/0xc7c > (XEN) [<00000000002001b4>] arm64/head.o#primary_switched+0xc/0x2c > (XEN) > (XEN) > (XEN) **************************************** > (XEN) Panic on CPU 0: > (XEN) Assertion 'unreachable' failed at > ...ild-workspace/build/xen/xen/include/xen/iommu.h:72 > (XEN) **************************************** > (XEN) > > > -- > Regards, > > Oleksandr Tyshchenko _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message----- > From: Paul Durrant > Sent: 25 September 2019 17:04 > To: 'Oleksandr' <olekstysh@gmail.com>; 'Jan Beulich' <jbeulich@suse.com> > Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu > <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George Dunlap > <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson > <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org; > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien Grall > <julien.grall@arm.com> > Subject: RE: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control > > > -----Original Message----- > > From: Oleksandr <olekstysh@gmail.com> > > Sent: 25 September 2019 16:50 > > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com> > > Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei > Liu > > <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper > > <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George > Dunlap > > <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson > > <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen- > devel@lists.xenproject.org; > > Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien Grall > > <julien.grall@arm.com> > > Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control > > > > > > [CC Julien] > > > > > > Hi Paul > > > > I may mistake, but looks like > > > > 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up > > iommu_use_hap_pt() and need_iommu_pt_sync() macros > > > > triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built > > with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) . > > > > So, iommu_setup() calls clear_iommu_hap_pt_share() with > > iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which, > > actually, triggers ASSERT. > > BTW, I assume disabling the iommu on the xen command like will work round the issue? Actually, no, that would be no good either. Paul > > Paul > > > > > ... > > > > > > (XEN) Assertion 'unreachable' failed at > > ...ild-workspace/build/xen/xen/include/xen/iommu.h:72 > > (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]---- > > (XEN) CPU: 0 > > (XEN) PC: 00000000002b3ae0 iommu_setup+0xa0/0x18c > > (XEN) LR: 00000000002b3a8c > > (XEN) SP: 00000000002f7dc0 > > (XEN) CPSR: a0000249 MODE:64-bit EL2h (Hypervisor, handler) > > (XEN) X0: 00000000002a7000 X1: 0000000000000000 X2: 2c736173656e6572 > > (XEN) X3: 0000000000000002 X4: 0000000000000001 X5: 0000000000000000 > > (XEN) X6: 0000000000000080 X7: 2b726072646d6471 X8: 7f7f7f7f7f7f7f7f > > (XEN) X9: ff65685e6c6f7275 X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101 > > (XEN) X12: 0000000000000038 X13: 0000000000280910 X14: 0000000000000020 > > (XEN) X15: 0000000000000000 X16: 00000000002a7000 X17: 00000000002a7000 > > (XEN) X18: 00000000002a7000 X19: 0000000000000000 X20: 00000000ffffffed > > (XEN) X21: 00000000002a6380 X22: 0000000000335430 X23: 0000000000000002 > > (XEN) X24: 000000000029b1f0 X25: 00000000002d83d0 X26: 0000000048000000 > > (XEN) X27: 00000000c0000000 X28: 0000000000000001 FP: 00000000002f7dc0 > > (XEN) > > (XEN) VTCR_EL2: 80000000 > > (XEN) VTTBR_EL2: 0000000000000000 > > (XEN) > > (XEN) SCTLR_EL2: 30cd183d > > (XEN) HCR_EL2: 0000000000000038 > > (XEN) TTBR0_EL2: 00000000781b4000 > > (XEN) > > (XEN) ESR_EL2: f2000001 > > (XEN) HPFAR_EL2: 0000000000000000 > > (XEN) FAR_EL2: 0000000000000000 > > (XEN) > > (XEN) Xen stack trace from sp=00000000002f7dc0: > > (XEN) 00000000002f7de0 00000000002bdd94 0000000000000002 0000000000000002 > > (XEN) 00000000bfe0b660 00000000002001b4 0000000078080000 0000000077e80000 > > (XEN) 0000000048000000 0000000000000000 0000000000400000 0000000000000003 > > (XEN) 0000000000000001 0000000000000000 0000000078080000 0000000048080040 > > (XEN) 0000000000000000 000000000000f080 0000000048000000 0000000078000000 > > (XEN) 00000000002d83c0 00000000002aa440 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000300000000 0000000000000000 00000040ffffffff > > (XEN) 0000000000000400 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) Xen call trace: > > (XEN) [<00000000002b3ae0>] iommu_setup+0xa0/0x18c (PC) > > (XEN) [<00000000002b3a8c>] iommu_setup+0x4c/0x18c (LR) > > (XEN) [<00000000002bdd94>] start_xen+0xaa0/0xc7c > > (XEN) [<00000000002001b4>] arm64/head.o#primary_switched+0xc/0x2c > > (XEN) > > (XEN) > > (XEN) **************************************** > > (XEN) Panic on CPU 0: > > (XEN) Assertion 'unreachable' failed at > > ...ild-workspace/build/xen/xen/include/xen/iommu.h:72 > > (XEN) **************************************** > > (XEN) > > > > > > -- > > Regards, > > > > Oleksandr Tyshchenko _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 25.09.19 19:03, Paul Durrant wrote: Hi, Paul >> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control >> >> >> [CC Julien] >> >> >> Hi Paul >> >> I may mistake, but looks like >> >> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up >> iommu_use_hap_pt() and need_iommu_pt_sync() macros >> >> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built >> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) . >> >> So, iommu_setup() calls clear_iommu_hap_pt_share() with >> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which, >> actually, triggers ASSERT. > BTW, I assume disabling the iommu on the xen command like will work round the issue? No. Disabling the iommu will lead to ASSERT in all cases. -- Regards, Oleksandr Tyshchenko _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message-----
> From: Oleksandr <olekstysh@gmail.com>
> Sent: 25 September 2019 16:50
> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com>
> Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George Dunlap
> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien Grall
> <julien.grall@arm.com>
> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
>
>
> [CC Julien]
>
>
> Hi Paul
>
> I may mistake, but looks like
>
> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
> iommu_use_hap_pt() and need_iommu_pt_sync() macros
>
> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) .
>
> So, iommu_setup() calls clear_iommu_hap_pt_share() with
> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
> actually, triggers ASSERT.
>
Here a minimal patch, leaving 'force pt share' in place. Does this avoid the problem?
---8<---
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index e8763c7fdf..f88a285e7f 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
pi->max_mfn = get_upper_mfn_bound();
arch_do_physinfo(pi);
if ( iommu_enabled )
+ {
pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
- if ( iommu_hap_pt_share )
- pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
+ if ( iommu_hap_pt_share )
+ pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
+ }
if ( copy_to_guest(u_sysctl, op, 1) )
ret = -EFAULT;
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 7c3003f3f1..6a10a24128 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
{
#ifndef iommu_hap_pt_share
iommu_hap_pt_share = false;
-#elif iommu_hap_pt_share
- ASSERT_UNREACHABLE();
#endif
}
---8<---
Paul
> ...
>
>
> (XEN) Assertion 'unreachable' failed at
> ...ild-workspace/build/xen/xen/include/xen/iommu.h:72
> (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]----
> (XEN) CPU: 0
> (XEN) PC: 00000000002b3ae0 iommu_setup+0xa0/0x18c
> (XEN) LR: 00000000002b3a8c
> (XEN) SP: 00000000002f7dc0
> (XEN) CPSR: a0000249 MODE:64-bit EL2h (Hypervisor, handler)
> (XEN) X0: 00000000002a7000 X1: 0000000000000000 X2: 2c736173656e6572
> (XEN) X3: 0000000000000002 X4: 0000000000000001 X5: 0000000000000000
> (XEN) X6: 0000000000000080 X7: 2b726072646d6471 X8: 7f7f7f7f7f7f7f7f
> (XEN) X9: ff65685e6c6f7275 X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101
> (XEN) X12: 0000000000000038 X13: 0000000000280910 X14: 0000000000000020
> (XEN) X15: 0000000000000000 X16: 00000000002a7000 X17: 00000000002a7000
> (XEN) X18: 00000000002a7000 X19: 0000000000000000 X20: 00000000ffffffed
> (XEN) X21: 00000000002a6380 X22: 0000000000335430 X23: 0000000000000002
> (XEN) X24: 000000000029b1f0 X25: 00000000002d83d0 X26: 0000000048000000
> (XEN) X27: 00000000c0000000 X28: 0000000000000001 FP: 00000000002f7dc0
> (XEN)
> (XEN) VTCR_EL2: 80000000
> (XEN) VTTBR_EL2: 0000000000000000
> (XEN)
> (XEN) SCTLR_EL2: 30cd183d
> (XEN) HCR_EL2: 0000000000000038
> (XEN) TTBR0_EL2: 00000000781b4000
> (XEN)
> (XEN) ESR_EL2: f2000001
> (XEN) HPFAR_EL2: 0000000000000000
> (XEN) FAR_EL2: 0000000000000000
> (XEN)
> (XEN) Xen stack trace from sp=00000000002f7dc0:
> (XEN) 00000000002f7de0 00000000002bdd94 0000000000000002 0000000000000002
> (XEN) 00000000bfe0b660 00000000002001b4 0000000078080000 0000000077e80000
> (XEN) 0000000048000000 0000000000000000 0000000000400000 0000000000000003
> (XEN) 0000000000000001 0000000000000000 0000000078080000 0000000048080040
> (XEN) 0000000000000000 000000000000f080 0000000048000000 0000000078000000
> (XEN) 00000000002d83c0 00000000002aa440 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000300000000 0000000000000000 00000040ffffffff
> (XEN) 0000000000000400 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> (XEN) Xen call trace:
> (XEN) [<00000000002b3ae0>] iommu_setup+0xa0/0x18c (PC)
> (XEN) [<00000000002b3a8c>] iommu_setup+0x4c/0x18c (LR)
> (XEN) [<00000000002bdd94>] start_xen+0xaa0/0xc7c
> (XEN) [<00000000002001b4>] arm64/head.o#primary_switched+0xc/0x2c
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Assertion 'unreachable' failed at
> ...ild-workspace/build/xen/xen/include/xen/iommu.h:72
> (XEN) ****************************************
> (XEN)
>
>
> --
> Regards,
>
> Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 25.09.19 19:10, Paul Durrant wrote:
Hi Paul
>> -----Original Message-----
>> From: Oleksandr <olekstysh@gmail.com>
>> Sent: 25 September 2019 16:50
>> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com>
>> Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
>> <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper
>> <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George Dunlap
>> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
>> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org;
>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien Grall
>> <julien.grall@arm.com>
>> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
>>
>>
>> [CC Julien]
>>
>>
>> Hi Paul
>>
>> I may mistake, but looks like
>>
>> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
>> iommu_use_hap_pt() and need_iommu_pt_sync() macros
>>
>> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
>> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) .
>>
>> So, iommu_setup() calls clear_iommu_hap_pt_share() with
>> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
>> actually, triggers ASSERT.
>>
> Here a minimal patch, leaving 'force pt share' in place. Does this avoid the problem?
>
> ---8<---
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index e8763c7fdf..f88a285e7f 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> pi->max_mfn = get_upper_mfn_bound();
> arch_do_physinfo(pi);
> if ( iommu_enabled )
> + {
> pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
> - if ( iommu_hap_pt_share )
> - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> + if ( iommu_hap_pt_share )
> + pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> + }
>
> if ( copy_to_guest(u_sysctl, op, 1) )
> ret = -EFAULT;
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 7c3003f3f1..6a10a24128 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
> {
> #ifndef iommu_hap_pt_share
> iommu_hap_pt_share = false;
> -#elif iommu_hap_pt_share
> - ASSERT_UNREACHABLE();
> #endif
> }
> ---8<---
>
> Paul
Thank you for the patch, but I need some time to check it (now I have
some troubles with starting guest VM). I will inform you once I check.
--
Regards,
Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi Paul
>>>
>>> I may mistake, but looks like
>>>
>>> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
>>> iommu_use_hap_pt() and need_iommu_pt_sync() macros
>>>
>>> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
>>> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not
>>> set) .
>>>
>>> So, iommu_setup() calls clear_iommu_hap_pt_share() with
>>> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
>>> actually, triggers ASSERT.
>>>
>> Here a minimal patch, leaving 'force pt share' in place. Does this
>> avoid the problem?
>>
>> ---8<---
>> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
>> index e8763c7fdf..f88a285e7f 100644
>> --- a/xen/common/sysctl.c
>> +++ b/xen/common/sysctl.c
>> @@ -268,9 +268,11 @@ long
>> do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>> pi->max_mfn = get_upper_mfn_bound();
>> arch_do_physinfo(pi);
>> if ( iommu_enabled )
>> + {
>> pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
>> - if ( iommu_hap_pt_share )
>> - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
>> + if ( iommu_hap_pt_share )
>> + pi->capabilities |=
>> XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
>> + }
>>
>> if ( copy_to_guest(u_sysctl, op, 1) )
>> ret = -EFAULT;
>> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
>> index 7c3003f3f1..6a10a24128 100644
>> --- a/xen/include/xen/iommu.h
>> +++ b/xen/include/xen/iommu.h
>> @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
>> {
>> #ifndef iommu_hap_pt_share
>> iommu_hap_pt_share = false;
>> -#elif iommu_hap_pt_share
>> - ASSERT_UNREACHABLE();
>> #endif
>> }
>> ---8<---
>>
>> Paul
>
> Thank you for the patch, but I need some time to check it (now I have
> some troubles with starting guest VM). I will inform you once I check.
With the patch applied, the issue I have faced (during Xen boot) is gone
away. But, I haven't analyzed your "per-domain IOMMU control series" yet
to have opinion regarding your patch itself.
I noticed the following:
When iommu_enabled = false (my case, when IOMMU driver is disable), I
can't create guest VM if "dtdev" property is present and contains DMA
masters in the domain config:
Parsing config from /xt/dom.cfg/domd.cfg
ERROR: passthrough not supported on this platform
Even if I add passthrough = "disabled", it still denies:
Parsing config from /xt/dom.cfg/domd.cfg
ERROR: passthrough disabled but devices are specified
Looks like, correct behavior...
--
Regards,
Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message-----
> From: Oleksandr <olekstysh@gmail.com>
> Sent: 25 September 2019 19:07
> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com>
> Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
> <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George Dunlap
> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien Grall
> <julien.grall@arm.com>
> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
>
>
> Hi Paul
>
>
> >>>
> >>> I may mistake, but looks like
> >>>
> >>> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
> >>> iommu_use_hap_pt() and need_iommu_pt_sync() macros
> >>>
> >>> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
> >>> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not
> >>> set) .
> >>>
> >>> So, iommu_setup() calls clear_iommu_hap_pt_share() with
> >>> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
> >>> actually, triggers ASSERT.
> >>>
> >> Here a minimal patch, leaving 'force pt share' in place. Does this
> >> avoid the problem?
> >>
> >> ---8<---
> >> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> >> index e8763c7fdf..f88a285e7f 100644
> >> --- a/xen/common/sysctl.c
> >> +++ b/xen/common/sysctl.c
> >> @@ -268,9 +268,11 @@ long
> >> do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> >> pi->max_mfn = get_upper_mfn_bound();
> >> arch_do_physinfo(pi);
> >> if ( iommu_enabled )
> >> + {
> >> pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
> >> - if ( iommu_hap_pt_share )
> >> - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> >> + if ( iommu_hap_pt_share )
> >> + pi->capabilities |=
> >> XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> >> + }
> >>
> >> if ( copy_to_guest(u_sysctl, op, 1) )
> >> ret = -EFAULT;
> >> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> >> index 7c3003f3f1..6a10a24128 100644
> >> --- a/xen/include/xen/iommu.h
> >> +++ b/xen/include/xen/iommu.h
> >> @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
> >> {
> >> #ifndef iommu_hap_pt_share
> >> iommu_hap_pt_share = false;
> >> -#elif iommu_hap_pt_share
> >> - ASSERT_UNREACHABLE();
> >> #endif
> >> }
> >> ---8<---
> >>
> >> Paul
> >
> > Thank you for the patch, but I need some time to check it (now I have
> > some troubles with starting guest VM). I will inform you once I check.
>
>
> With the patch applied, the issue I have faced (during Xen boot) is gone
> away. But, I haven't analyzed your "per-domain IOMMU control series" yet
> to have opinion regarding your patch itself.
>
>
> I noticed the following:
>
> When iommu_enabled = false (my case, when IOMMU driver is disable), I
> can't create guest VM if "dtdev" property is present and contains DMA
> masters in the domain config:
>
> Parsing config from /xt/dom.cfg/domd.cfg
> ERROR: passthrough not supported on this platform
>
> Even if I add passthrough = "disabled", it still denies:
>
> Parsing config from /xt/dom.cfg/domd.cfg
> ERROR: passthrough disabled but devices are specified
>
> Looks like, correct behavior...
>
Yes, that all seems correct. Passthrough should not be done without an IOMMU.
Paul
>
> --
> Regards,
>
> Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi,
On 25/09/2019 17:10, Paul Durrant wrote:
>> -----Original Message-----
>> From: Oleksandr <olekstysh@gmail.com>
>> Sent: 25 September 2019 16:50
>> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com>
>> Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei Liu
>> <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper
>> <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George Dunlap
>> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
>> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org;
>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien Grall
>> <julien.grall@arm.com>
>> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
>>
>>
>> [CC Julien]
>>
>>
>> Hi Paul
>>
>> I may mistake, but looks like
>>
>> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
>> iommu_use_hap_pt() and need_iommu_pt_sync() macros
>>
>> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
>> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) .
>>
>> So, iommu_setup() calls clear_iommu_hap_pt_share() with
>> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
>> actually, triggers ASSERT.
>>
>
> Here a minimal patch, leaving 'force pt share' in place. Does this avoid the problem?
>
> ---8<---
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index e8763c7fdf..f88a285e7f 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> pi->max_mfn = get_upper_mfn_bound();
> arch_do_physinfo(pi);
> if ( iommu_enabled )
> + {
> pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
> - if ( iommu_hap_pt_share )
> - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> + if ( iommu_hap_pt_share )
> + pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> + }
>
> if ( copy_to_guest(u_sysctl, op, 1) )
> ret = -EFAULT;
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 7c3003f3f1..6a10a24128 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
> {
> #ifndef iommu_hap_pt_share
> iommu_hap_pt_share = false;
> -#elif iommu_hap_pt_share
> - ASSERT_UNREACHABLE();
> #endif
IHMO, calling this function is a mistake on platform only supporting
shared page-table so the ASSERT() should be kept here.
This raises the question why the function is actually called from common
code. iommu_hap_enabled() should technically not be used in any code if
the IOMMU is not enabled/present. So what are you trying to prevent here?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message-----
> From: Julien Grall <Julien.Grall@arm.com>
> Sent: 25 September 2019 22:34
> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Oleksandr' <olekstysh@gmail.com>; 'Jan Beulich'
> <jbeulich@suse.com>
> Cc: nd <nd@arm.com>; Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George
> Dunlap <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
>
> Hi,
>
> On 25/09/2019 17:10, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Oleksandr <olekstysh@gmail.com>
> >> Sent: 25 September 2019 16:50
> >> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com>
> >> Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei
> Liu
> >> <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper
> >> <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George
> Dunlap
> >> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
> >> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-
> devel@lists.xenproject.org;
> >> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien
> Grall
> >> <julien.grall@arm.com>
> >> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
> >>
> >>
> >> [CC Julien]
> >>
> >>
> >> Hi Paul
> >>
> >> I may mistake, but looks like
> >>
> >> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
> >> iommu_use_hap_pt() and need_iommu_pt_sync() macros
> >>
> >> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
> >> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) .
> >>
> >> So, iommu_setup() calls clear_iommu_hap_pt_share() with
> >> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
> >> actually, triggers ASSERT.
> >>
> >
> > Here a minimal patch, leaving 'force pt share' in place. Does this avoid the problem?
> >
> > ---8<---
> > diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> > index e8763c7fdf..f88a285e7f 100644
> > --- a/xen/common/sysctl.c
> > +++ b/xen/common/sysctl.c
> > @@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> > pi->max_mfn = get_upper_mfn_bound();
> > arch_do_physinfo(pi);
> > if ( iommu_enabled )
> > + {
> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
> > - if ( iommu_hap_pt_share )
> > - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> > + if ( iommu_hap_pt_share )
> > + pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> > + }
> >
> > if ( copy_to_guest(u_sysctl, op, 1) )
> > ret = -EFAULT;
> > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> > index 7c3003f3f1..6a10a24128 100644
> > --- a/xen/include/xen/iommu.h
> > +++ b/xen/include/xen/iommu.h
> > @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
> > {
> > #ifndef iommu_hap_pt_share
> > iommu_hap_pt_share = false;
> > -#elif iommu_hap_pt_share
> > - ASSERT_UNREACHABLE();
> > #endif
>
> IHMO, calling this function is a mistake on platform only supporting
> shared page-table so the ASSERT() should be kept here.
>
> This raises the question why the function is actually called from common
> code. iommu_hap_enabled() should technically not be used in any code if
> the IOMMU is not enabled/present. So what are you trying to prevent here?
What I'm trying to prevent, on x86, is a situation where the iommu_enabled == false but iommu_hap_pt_share == true. I had, mistakenly, believed that iommu_enabled would never be false for ARM but it seems this is not the case so that situation has to be tolerated. I guess, given the other hunk of my patch, I can actually leave the ASSERT in place and avoid making the call from common code, in which case the function ought to move into an x86 header as well.
Paul
>
> Cheers,
>
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi,
On 9/26/19 9:39 AM, Paul Durrant wrote:
>> -----Original Message-----
>> From: Julien Grall <Julien.Grall@arm.com>
>> Sent: 25 September 2019 22:34
>> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Oleksandr' <olekstysh@gmail.com>; 'Jan Beulich'
>> <jbeulich@suse.com>
>> Cc: nd <nd@arm.com>; Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini
>> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew
>> Cooper <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George
>> Dunlap <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
>> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org;
>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>
>> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
>>
>> Hi,
>>
>> On 25/09/2019 17:10, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Oleksandr <olekstysh@gmail.com>
>>>> Sent: 25 September 2019 16:50
>>>> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com>
>>>> Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>; Wei
>> Liu
>>>> <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper
>>>> <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George
>> Dunlap
>>>> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
>>>> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-
>> devel@lists.xenproject.org;
>>>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien
>> Grall
>>>> <julien.grall@arm.com>
>>>> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
>>>>
>>>>
>>>> [CC Julien]
>>>>
>>>>
>>>> Hi Paul
>>>>
>>>> I may mistake, but looks like
>>>>
>>>> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
>>>> iommu_use_hap_pt() and need_iommu_pt_sync() macros
>>>>
>>>> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
>>>> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) .
>>>>
>>>> So, iommu_setup() calls clear_iommu_hap_pt_share() with
>>>> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
>>>> actually, triggers ASSERT.
>>>>
>>>
>>> Here a minimal patch, leaving 'force pt share' in place. Does this avoid the problem?
>>>
>>> ---8<---
>>> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
>>> index e8763c7fdf..f88a285e7f 100644
>>> --- a/xen/common/sysctl.c
>>> +++ b/xen/common/sysctl.c
>>> @@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>>> pi->max_mfn = get_upper_mfn_bound();
>>> arch_do_physinfo(pi);
>>> if ( iommu_enabled )
>>> + {
>>> pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
>>> - if ( iommu_hap_pt_share )
>>> - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
>>> + if ( iommu_hap_pt_share )
>>> + pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
>>> + }
>>>
>>> if ( copy_to_guest(u_sysctl, op, 1) )
>>> ret = -EFAULT;
>>> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
>>> index 7c3003f3f1..6a10a24128 100644
>>> --- a/xen/include/xen/iommu.h
>>> +++ b/xen/include/xen/iommu.h
>>> @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
>>> {
>>> #ifndef iommu_hap_pt_share
>>> iommu_hap_pt_share = false;
>>> -#elif iommu_hap_pt_share
>>> - ASSERT_UNREACHABLE();
>>> #endif
>>
>> IHMO, calling this function is a mistake on platform only supporting
>> shared page-table so the ASSERT() should be kept here.
>>
>> This raises the question why the function is actually called from common
>> code. iommu_hap_enabled() should technically not be used in any code if
>> the IOMMU is not enabled/present. So what are you trying to prevent here?
>
> What I'm trying to prevent, on x86, is a situation where the iommu_enabled == false but iommu_hap_pt_share == true.
This is not entirely uncommon to have other variables gated by others.
So what could happen if you have iommu_enabled == false and
iommu_hap_pt_share == true on x86?
> I had, mistakenly, believed that iommu_enabled would never be false for ARM but it seems this is not the case so that situation has to be tolerated. I guess, given the other hunk of my patch, I can actually leave the ASSERT in place and avoid making the call from common code, in which case the function ought to move into an x86 header as well.
By "the function", do you mean clear_iommu_hap_pt_share? If so, I think
it should stay were it is. This is a generic function that might be
re-used for other architecture in the future.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message-----
> From: Julien Grall <julien.grall@arm.com>
> Sent: 26 September 2019 10:13
> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Oleksandr' <olekstysh@gmail.com>; 'Jan Beulich'
> <jbeulich@suse.com>
> Cc: nd <nd@arm.com>; Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew
> Cooper <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George
> Dunlap <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-devel@lists.xenproject.org;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
>
> Hi,
>
> On 9/26/19 9:39 AM, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Julien Grall <Julien.Grall@arm.com>
> >> Sent: 25 September 2019 22:34
> >> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Oleksandr' <olekstysh@gmail.com>; 'Jan Beulich'
> >> <jbeulich@suse.com>
> >> Cc: nd <nd@arm.com>; Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini
> >> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>;
> Andrew
> >> Cooper <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>;
> George
> >> Dunlap <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
> >> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-
> devel@lists.xenproject.org;
> >> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>
> >> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
> >>
> >> Hi,
> >>
> >> On 25/09/2019 17:10, Paul Durrant wrote:
> >>>> -----Original Message-----
> >>>> From: Oleksandr <olekstysh@gmail.com>
> >>>> Sent: 25 September 2019 16:50
> >>>> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' <jbeulich@suse.com>
> >>>> Cc: Petre Pircalabu <ppircalabu@bitdefender.com>; Stefano Stabellini <sstabellini@kernel.org>;
> Wei
> >> Liu
> >>>> <wl@xen.org>; KonradRzeszutek Wilk <konrad.wilk@oracle.com>; Andrew Cooper
> >>>> <Andrew.Cooper3@citrix.com>; David Scott <dave@recoil.org>; Tim (Xen.org) <tim@xen.org>; George
> >> Dunlap
> >>>> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Ian Jackson
> >>>> <Ian.Jackson@citrix.com>; Anthony Perard <anthony.perard@citrix.com>; xen-
> >> devel@lists.xenproject.org;
> >>>> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monne <roger.pau@citrix.com>; Julien
> >> Grall
> >>>> <julien.grall@arm.com>
> >>>> Subject: Re: [Xen-devel] [PATCH v13 0/4] add per-domain IOMMU control
> >>>>
> >>>>
> >>>> [CC Julien]
> >>>>
> >>>>
> >>>> Hi Paul
> >>>>
> >>>> I may mistake, but looks like
> >>>>
> >>>> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
> >>>> iommu_use_hap_pt() and need_iommu_pt_sync() macros
> >>>>
> >>>> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
> >>>> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) .
> >>>>
> >>>> So, iommu_setup() calls clear_iommu_hap_pt_share() with
> >>>> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
> >>>> actually, triggers ASSERT.
> >>>>
> >>>
> >>> Here a minimal patch, leaving 'force pt share' in place. Does this avoid the problem?
> >>>
> >>> ---8<---
> >>> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> >>> index e8763c7fdf..f88a285e7f 100644
> >>> --- a/xen/common/sysctl.c
> >>> +++ b/xen/common/sysctl.c
> >>> @@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> >>> pi->max_mfn = get_upper_mfn_bound();
> >>> arch_do_physinfo(pi);
> >>> if ( iommu_enabled )
> >>> + {
> >>> pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
> >>> - if ( iommu_hap_pt_share )
> >>> - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> >>> + if ( iommu_hap_pt_share )
> >>> + pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
> >>> + }
> >>>
> >>> if ( copy_to_guest(u_sysctl, op, 1) )
> >>> ret = -EFAULT;
> >>> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> >>> index 7c3003f3f1..6a10a24128 100644
> >>> --- a/xen/include/xen/iommu.h
> >>> +++ b/xen/include/xen/iommu.h
> >>> @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
> >>> {
> >>> #ifndef iommu_hap_pt_share
> >>> iommu_hap_pt_share = false;
> >>> -#elif iommu_hap_pt_share
> >>> - ASSERT_UNREACHABLE();
> >>> #endif
> >>
> >> IHMO, calling this function is a mistake on platform only supporting
> >> shared page-table so the ASSERT() should be kept here.
> >>
> >> This raises the question why the function is actually called from common
> >> code. iommu_hap_enabled() should technically not be used in any code if
> >> the IOMMU is not enabled/present. So what are you trying to prevent here?
> >
> > What I'm trying to prevent, on x86, is a situation where the iommu_enabled == false but
> iommu_hap_pt_share == true.
>
> This is not entirely uncommon to have other variables gated by others.
> So what could happen if you have iommu_enabled == false and
> iommu_hap_pt_share == true on x86?
>
Well, I was hoping to avoid the nested if in sysctl.c.
> > I had, mistakenly, believed that iommu_enabled would never be false for ARM but it seems this is not
> the case so that situation has to be tolerated. I guess, given the other hunk of my patch, I can
> actually leave the ASSERT in place and avoid making the call from common code, in which case the
> function ought to move into an x86 header as well.
>
> By "the function", do you mean clear_iommu_hap_pt_share?
Yes.
> If so, I think
> it should stay were it is. This is a generic function that might be
> re-used for other architecture in the future.
>
That seems like a bit of a long shot. If I remove the call from iommu_setup() then the only remaining callers are in x86 code, but I suppose it can stay where it is to avoid the churn. I'll spin a new test patch.
Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi Paul, On 9/26/19 10:17 AM, Paul Durrant wrote: >> If so, I think >> it should stay were it is. This is a generic function that might be >> re-used for other architecture in the future. >> > > That seems like a bit of a long shot. If I remove the call from iommu_setup() then the only remaining callers are in x86 code, but I suppose it can stay where it is to avoid the churn. I'll spin a new test patch. Not really, I know that we will likely need it on Arm when MSI doorbell will be exposed to the guest because some mappings cannot be accessed by the processor. Although, I can't tell when this will happen. Anyway, I will have a look at your next patch :). Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 26.09.19 11:39, Paul Durrant wrote:
Hi Paul
>>
>>>> [CC Julien]
>>>>
>>>>
>>>> Hi Paul
>>>>
>>>> I may mistake, but looks like
>>>>
>>>> 80ff3d338dc93260b41ffeeebb0f852c2edef9ce iommu: tidy up
>>>> iommu_use_hap_pt() and need_iommu_pt_sync() macros
>>>>
>>>> triggers ASSERT_UNREACHABLE on Arm if no IOMMU has been found (I built
>>>> with my platform's IOMMU driver disabled: # CONFIG_IPMMU_VMSA is not set) .
>>>>
>>>> So, iommu_setup() calls clear_iommu_hap_pt_share() with
>>>> iommu_hap_pt_share being set (CONFIG_IOMMU_FORCE_PT_SHARE=y) which,
>>>> actually, triggers ASSERT.
>>>>
>>> Here a minimal patch, leaving 'force pt share' in place. Does this avoid the problem?
>>>
>>> ---8<---
>>> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
>>> index e8763c7fdf..f88a285e7f 100644
>>> --- a/xen/common/sysctl.c
>>> +++ b/xen/common/sysctl.c
>>> @@ -268,9 +268,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>>> pi->max_mfn = get_upper_mfn_bound();
>>> arch_do_physinfo(pi);
>>> if ( iommu_enabled )
>>> + {
>>> pi->capabilities |= XEN_SYSCTL_PHYSCAP_directio;
>>> - if ( iommu_hap_pt_share )
>>> - pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
>>> + if ( iommu_hap_pt_share )
>>> + pi->capabilities |= XEN_SYSCTL_PHYSCAP_iommu_hap_pt_share;
>>> + }
>>>
>>> if ( copy_to_guest(u_sysctl, op, 1) )
>>> ret = -EFAULT;
>>> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
>>> index 7c3003f3f1..6a10a24128 100644
>>> --- a/xen/include/xen/iommu.h
>>> +++ b/xen/include/xen/iommu.h
>>> @@ -68,8 +68,6 @@ static inline void clear_iommu_hap_pt_share(void)
>>> {
>>> #ifndef iommu_hap_pt_share
>>> iommu_hap_pt_share = false;
>>> -#elif iommu_hap_pt_share
>>> - ASSERT_UNREACHABLE();
>>> #endif
>> IHMO, calling this function is a mistake on platform only supporting
>> shared page-table so the ASSERT() should be kept here.
>>
>> This raises the question why the function is actually called from common
>> code. iommu_hap_enabled() should technically not be used in any code if
>> the IOMMU is not enabled/present. So what are you trying to prevent here?
> What I'm trying to prevent, on x86, is a situation where the iommu_enabled == false but iommu_hap_pt_share == true. I had, mistakenly, believed that iommu_enabled would never be false for ARM but it seems this is not the case so that situation has to be tolerated. I guess, given the other hunk of my patch, I can actually leave the ASSERT in place and avoid making the call from common code, in which case the function ought to move into an x86 header as well.
Not all Arm based SoCs (which supported by Xen) contains SMMU (the only
one supported driver at the moment) or IPMMU-VMSA (on review now, but,
it will be under CONFIG_EXPERT when merged, so disabled by default). So,
"iommu_enabled" can be false.
--
Regards,
Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.