Re: [PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit
Hi Julien, On 21.02.2022 16:58, Julien Grall wrote: > Hi Michal, > > On 21/02/2022 10:59, Michal Orzel wrote: >> Following a discussion [1] it seems like that renaming work has >> been forgotten. > > This is in my todo list of clean-up I need to do for Xen. But I haven't yet > had a chance to look at it. Thank you for taking a look! > >> Perform renaming of psr_mode_is_32bit to >> regs_mode_is_32bit as the function no longer takes psr parameter. > > If we modify psr_mode_is_32bit(), then we should also modify > psr_mode_is_user() because they have the same prototype and we should keep > the naming consistent. > Ok, I agree. Do you think this should be done in a separate patch? FWICS, psr_mode_is_user is used in traps.c, vcpreg.c ,vtimer.c and vsysreg.c whereas psr_mode_is_32bit - only in traps.c. >> >> [1] https://marc.info/?l=xen-devel&m=156457538423787&w=2 > > NIT: The first sentence and this link adds value for the review on the > mailing list (we know where the request came from) but doesn't add any after > the commit message (there are no extra information in them). > > So I would move this information after ---. This will get dropped on commit. > Ok. > Cheers, > Cheers, Michal
[linux-linus test] 168187: tolerable FAIL - PUSHED
flight 168187 linux-linus real [real] flight 168190 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/168187/ http://logs.test-lab.xenproject.org/osstest/logs/168190/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass in 168190-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 168183 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 168183 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168183 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168183 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 168183 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168183 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 168183 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 168183 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass version targeted for testing: linux038101e6b2cd5c55f888f85db42ea2ad3aecb4b6 baseline version: linuxcfb92440ee71adcc2105b0890bb01ac3cddb8507 Last test of basis 168183 2022-02-21 02:00:50 Z1 days Testing same since 168187 2022-02-21 17:39:52 Z0 days1 attempts People who touched revisions under test: Daniel Scally Hans de Goede Linus Torvalds jobs: build-amd64-xsm pass buil
[xen-unstable-smoke test] 168188: tolerable all pass - PUSHED
flight 168188 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/168188/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 95d9ab46143685f169f636cfdd7997e2fc630e86 baseline version: xen 686f13cfce1d95464ff39fb59ac1f85163cea03b Last test of basis 168167 2022-02-18 19:01:37 Z3 days Testing same since 168188 2022-02-21 21:01:44 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Roger Pau Monne Roger Pau Monné jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 686f13cfce..95d9ab4614 95d9ab46143685f169f636cfdd7997e2fc630e86 -> smoke
[qemu-mainline test] 168186: tolerable FAIL - PUSHED
flight 168186 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/168186/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168180 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 168180 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168180 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 168180 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 168180 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 168180 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 168180 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168180 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: qemuu477c3b934a47adf7de285863f59d6e4503dd1a6d baseline version: qemuue670f6d825d4dee248b311197fd4048469d6772b Last test of basis 168180 2022-02-20 22:09:39 Z1 days Testing same since 168186 2022-02-21 17:36:59 Z0 days1 attempts People who touched revisions under test: Akihiko Odaki Alexander Graf Ani Sinha Bern
[PATCH v2 1/2] Revert "xen-netback: remove 'hotplug-status' once it has served its purpose"
This reverts commit 1f2565780e9b7218cf92c7630130e82dcc0fe9c2. The 'hotplug-status' node should not be removed as long as the vif device remains configured. Otherwise the xen-netback would wait for re-running the network script even if it was already called (in case of the frontent re-connecting). But also, it _should_ be removed when the vif device is destroyed (for example when unbinding the driver) - otherwise hotplug script would not configure the device whenever it re-appear. Moving removal of the 'hotplug-status' node was a workaround for nothing calling network script after xen-netback module is reloaded. But when vif interface is re-created (on xen-netback unbind/bind for example), the script should be called, regardless of who does that - currently this case is not handled by the toolstack, and requires manual script call. Keeping hotplug-status=connected to skip the call is wrong and leads to not configured interface. More discussion at https://lore.kernel.org/xen-devel/afedd7cb-a291-e773-8b0d-4db9b291f...@ipxe.org/T/#u Cc: sta...@vger.kernel.org Signed-off-by: Marek Marczykowski-Górecki --- Cc: Michael Brown Changes in v2: - build fix, reported by kernel test robot --- drivers/net/xen-netback/xenbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index d24b7a7993aa..3fad58d22155 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -256,6 +256,7 @@ static void backend_disconnect(struct backend_info *be) unsigned int queue_index; xen_unregister_watchers(vif); + xenbus_rm(XBT_NIL, be->dev->nodename, "hotplug-status"); #ifdef CONFIG_DEBUG_FS xenvif_debugfs_delif(vif); #endif /* CONFIG_DEBUG_FS */ @@ -675,7 +676,6 @@ static void hotplug_status_changed(struct xenbus_watch *watch, /* Not interested in this watch anymore. */ unregister_hotplug_status_watch(be); - xenbus_rm(XBT_NIL, be->dev->nodename, "hotplug-status"); } kfree(str); } -- 2.31.1
[PATCH v2 2/2] Revert "xen-netback: Check for hotplug-status existence before watching"
This reverts commit 2afeec08ab5c86ae21952151f726bfe184f6b23d.
The reasoning in the commit was wrong - the code expected to setup the
watch even if 'hotplug-status' didn't exist. In fact, it relied on the
watch being fired the first time - to check if maybe 'hotplug-status' is
already set to 'connected'. Not registering a watch for non-existing
path (which is the case if hotplug script hasn't been executed yet),
made the backend not waiting for the hotplug script to execute. This in
turns, made the netfront think the interface is fully operational, while
in fact it was not (the vif interface on xen-netback side might not be
configured yet).
This was a workaround for 'hotplug-status' erroneously being removed.
But since that is reverted now, the workaround is not necessary either.
More discussion at
https://lore.kernel.org/xen-devel/afedd7cb-a291-e773-8b0d-4db9b291f...@ipxe.org/T/#u
Cc: sta...@vger.kernel.org
Signed-off-by: Marek Marczykowski-Górecki
---
I believe this is the same issue as discussed at
https://lore.kernel.org/xen-devel/20220113111946.ga4133...@dingwall.me.uk/
Cc: James Dingwall
---
drivers/net/xen-netback/xenbus.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 3fad58d22155..990360d75cb6 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -824,15 +824,11 @@ static void connect(struct backend_info *be)
xenvif_carrier_on(be->vif);
unregister_hotplug_status_watch(be);
- if (xenbus_exists(XBT_NIL, dev->nodename, "hotplug-status")) {
- err = xenbus_watch_pathfmt(dev, &be->hotplug_status_watch,
- NULL, hotplug_status_changed,
- "%s/%s", dev->nodename,
- "hotplug-status");
- if (err)
- goto err;
+ err = xenbus_watch_pathfmt(dev, &be->hotplug_status_watch, NULL,
+ hotplug_status_changed,
+ "%s/%s", dev->nodename, "hotplug-status");
+ if (!err)
be->have_hotplug_status_watch = 1;
- }
netif_tx_wake_all_queues(be->vif->dev);
--
2.31.1
Re: [PATCH] xen/netfront: destroy queues before real_num_tx_queues is zeroed
On Mon, Feb 21, 2022 at 07:27:32AM +0100, Juergen Gross wrote: > I checked some of the call paths leading to xennet_close(), and all of > those contained an ASSERT_RTNL(), so it seems the rtnl_lock is already > taken here. Could you test with adding an ASSERT_RTNL() in > xennet_destroy_queues()? Tried that and no issues spotted. > In case your test with the added ASSERT_RTNL() doesn't show any > problem you can add my: > > Reviewed-by: Juergen Gross Thanks. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab signature.asc Description: PGP signature
Re: [XEN v8 2/2] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler
Hi Julien,
On 21/02/2022 19:13, Julien Grall wrote:
On 21/02/2022 19:05, Ayan Kumar Halder wrote:
If we (ie Xen) didn't decode the instruction manually, then
check_p2m() has not been invoked yet. This is because of the
following (info.dabt.valid == True) :-
if ( !is_data || !info.dabt.valid )
{
...
if ( check_p2m(is_data, gpa) )
return;
...
}
So, in this scenario ( !info.dabt.valid), it would not be necessary
to invoke check_p2m() after try_handle_mmio().
However, if we havenot decoded the instruction manually (ie
info.dabt.valid == True), and try_handle_mmio() returns IO_UNHANDLED,
then it will be necessary to invoke "check_p2m(is_data, gpa)"
Hmmm you are right. But this doesn't seem to match the code you wrote
below. What did I miss?
My code was not correct. I have rectified it as below. Please let me
know if it looks sane.
case FSC_FLT_TRANS:
{
info.gpa = gpa;
info.dabt = hsr.dabt;
/*
* Assumption :- Most of the times when we get a data abort and
the ISS
* is invalid or an instruction abort, the underlying cause is
that the
* page tables have not been set up correctly.
*/
if ( !is_data || !info.dabt.valid )
{
if ( check_p2m(is_data, gpa) )
return;
/*
* If the instruction abort could not be resolved by
setting the
* appropriate bits in the translation table, then Xen should
* forward the abort to the guest.
*/
if ( !is_data )
goto inject_abt;
try_decode_instruction(regs, &info);
/*
* If Xen could not decode the instruction or encountered
an error
* while decoding, then it should forward the abort to the
guest.
*/
if ( info.dabt_instr.state == INSTR_ERROR )
goto inject_abt;
}
state = try_handle_mmio(regs, &info);
switch ( state )
{
case IO_ABORT:
goto inject_abt;
case IO_HANDLED:
/*
* If the instruction was decoded and has executed
successfully
* on the MMIO region, then Xen should execute the next
part of
* the instruction. (for eg increment the rn if it is a
* post-indexing instruction.
*/
post_increment_register(&info.dabt_instr);
advance_pc(regs, hsr);
return;
case IO_RETRY:
/* finish later */
return;
case IO_UNHANDLED:
/* IO unhandled, try another way to handle it. */
break;
}
/*
* If the instruction was valid but Xen could not emulate the
instruction
* then it should configure the page tables to set the correct
page table
* entry corresponding to the faulting address. If it was
successful, it
* should return to the guest to retry the instruction (hoping
that the
* instruction will not be trapped to Xen again).
* However, if Xen was not successful in setting the page
tables, then
* it should forward the abort to the guest.
*/
if ( info.dabt.valid && check_p2m(is_data, gpa) )
return;
break;
}
default:
- Ayan
Cheers,
Re: [PATCH v2 04/70] x86/pv-shim: Don't modify the hypercall table
On 17/02/2022 10:20, Jan Beulich wrote: > On 16.02.2022 23:17, Andrew Cooper wrote: >> On 14/02/2022 13:56, Jan Beulich wrote: >>> On 14.02.2022 14:50, Andrew Cooper wrote: On 14/02/2022 13:33, Jan Beulich wrote: > On 14.02.2022 13:50, Andrew Cooper wrote: >> From: Juergen Gross >> >> When running as pv-shim the hypercall is modified today in order to >> replace the functions for __HYPERVISOR_event_channel_op and >> __HYPERVISOR_grant_table_op hypercalls. >> >> Change this to call the related functions from the normal handlers >> instead when running as shim. The performance implications are not >> really relevant, as a normal production hypervisor will not be >> configured to support shim mode, so the related calls will be dropped >> due to optimization of the compiler. >> >> Note that for the CONFIG_PV_SHIM_EXCLUSIVE case there is a dummy >> wrapper do_grant_table_op() needed, as in this case grant_table.c >> isn't being built. >> >> Signed-off-by: Juergen Gross >> Signed-off-by: Andrew Cooper > I don't think you sync-ed this with Jürgen's v3. There were only minor > changes but having a stale version sent two months later isn't very > nice. I did resync. What do you think is missing? >>> A few likely() / unlikely() as far as I could see. >> Oh those two. I appear to have forgot to email. >> >> They're wrong - observe they're in an ifndef block, not an ifdef block. > I don't see how the (unrelated) #ifndef matters here: The #ifndef > is about grant table availability. The two likely() are about > running as shim. I'm of the firm opinion that a binary built > without PV_SHIM_EXCLUSIVE is far more likely to be used as a bare > metal hypervisor. And for a PV_SHIM_EXCLUSIVE hypervisor the > conditions are constant anyway, and hence the unlikely() has no > effect. > > And if your way should really be followed, why did you deem the two > unlikely() in do_event_channel_op() and do_grant_table_op() okay? Because those are at least not incorrect. (I still think we have far too many annotations, and I doubt they're all helpful.) The gnttab stubs in the !GNTTAB case exist strictly for compile tests (there's no such thing as a production build of Xen without grant tables) and PV_SHIM_EXCLUSIVE builds. Code layout only matters for cases where we're executing code, which is the PV Shim case, at which point the condition is constant and doesn't generate a branch. A compiler ought to raise a warning on finding that __builtin_expect() has a constant parameter, because it's a nop in one case, and demonstrably false in the other. As for the function in question, the compiled result is an unconditional tailcall to pv_shim_grant_table_op. ~Andrew
Re: [XEN v8 2/2] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler
On 21/02/2022 19:05, Ayan Kumar Halder wrote:
If we (ie Xen) didn't decode the instruction manually, then check_p2m()
has not been invoked yet. This is because of the following
(info.dabt.valid == True) :-
if ( !is_data || !info.dabt.valid )
{
...
if ( check_p2m(is_data, gpa) )
return;
...
}
So, in this scenario ( !info.dabt.valid), it would not be necessary to
invoke check_p2m() after try_handle_mmio().
However, if we havenot decoded the instruction manually (ie
info.dabt.valid == True), and try_handle_mmio() returns IO_UNHANDLED,
then it will be necessary to invoke "check_p2m(is_data, gpa)"
Hmmm you are right. But this doesn't seem to match the code you wrote
below. What did I miss?
Cheers,
--
Julien Grall
Re: [XEN v8 2/2] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler
On 21/02/2022 17:57, Julien Grall wrote:
On 21/02/2022 17:05, Ayan Kumar Halder wrote:
Hi Julien,
Hi,
Hi Julien,
On 13/02/2022 12:19, Julien Grall wrote:
}
void register_mmio_handler(struct domain *d,
diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
index 308650b400..3c0a935ccf 100644
--- a/xen/arch/arm/ioreq.c
+++ b/xen/arch/arm/ioreq.c
@@ -47,6 +47,7 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs
*regs,
struct vcpu *v, mmio_info_t *info)
{
struct vcpu_io *vio = &v->io;
+ struct dabt_instr instr = info->dabt_instr;
ioreq_t p = {
.type = IOREQ_TYPE_COPY,
.addr = info->gpa,
@@ -76,10 +77,8 @@ enum io_state try_fwd_ioserv(struct
cpu_user_regs *regs,
if ( !s )
return IO_UNHANDLED;
- if ( !info->dabt.valid )
- return IO_ABORT;
-
For this one, I would switch to ASSERT(dabt.valid);
I see that try_fwd_ioserv() is invoked from try_handle_mmio() only.
Thus, if I follow your suggestion of adding a check for dabt.valid at
the beginning of try_handle_mmio(), then this ASSERT() is not required.
I agree that try_handle_mmio() is the only caller today. But we don't
know how this is going to be used tomorrow.
The goal of this ASSERT() is to catch those new users that would call
it wrongly.
[...]
... this will inject a data abort to the guest when we can't decode.
This is not what we want. We should check whether this is a P2M
translation fault or we need to map an MMIO region.
In pseudo-code, this would look like:
if ( !is_data || hsr.dabt.valid )
I think you mean if ( !is_data || !hsr.dabt.valid )
You are right.
The reason being if there is an instruction abort or a data abort
(with ISV == 0), then it should try to configure the page tables.
{
if ( check_p2m() )
return;
if ( !is_data )
goto inject_dabt;
decode_instruction();
if ( !dabt.invalid )
goto inject_dabt;
}
try_handle_mmio();
if ( instruction was not decoded )
check_p2m();
If the instruction was not decoded, then there is no need to
configure the page tables again. We have already done this before.
H... I think there are confusing about which sort of decoding I
was referring to. In this case, I mean if we didn't decode the
instruction manully, then it is not necessary to call check_p2m().
Do you agree with that?
If we (ie Xen) didn't decode the instruction manually, then check_p2m()
has not been invoked yet. This is because of the following
(info.dabt.valid == True) :-
if ( !is_data || !info.dabt.valid )
{
...
if ( check_p2m(is_data, gpa) )
return;
...
}
So, in this scenario ( !info.dabt.valid), it would not be necessary to
invoke check_p2m() after try_handle_mmio().
However, if we havenot decoded the instruction manually (ie
info.dabt.valid == True), and try_handle_mmio() returns IO_UNHANDLED,
then it will be necessary to invoke "check_p2m(is_data, gpa)"
- Ayan
So my understanding is as follows :-
/* Check that it is instruction abort or ISS is invalid. */
I have had a remark on this line before. Please have a look and
address it.
if ( !is_data || !info.dabt.valid )
{
/*
* If the instruction was trapped due to access to stage
1 translation
* then Xen should try to resolve the page table entry
for the stage 1
* translation table with the assumption that the page
tables are
* present in the non MMIO region. If it is successful,
then it should
* ask the guest to retry the instruction.
*/
I agree that we want to skip the MMIO mapping when s1ptw == 1.
However, I am not sure this belongs to this patch because this is
technically already a bug.
if ( is_data && info.dabt.s1ptw )
{
info.dabt_instr.state = INSTR_RETRY;
/* The translation tables are assumed to be in non
MMIO region. */
is_data = false;
is_data is also used to decide which sort of abort we want to send to
the guest (see after inject_dabt). So I don't think we could force set
is_data here.
Instead, I would define a new local variable (maybe
mmio_access_allowed) that will be set for instruction abort or when
s1ptw is 1.
}
/*
* Assumption :- Most of the times when we get a
translation fault
* and the ISS is invalid, the underlying cause is that
the page
* tables have not been set up correctly.
*/
I think this comment make more sense on top of "if !is_data ||
!info.dabt.valid".
if ( check_p2m(is_data, gpa) )
return;
/*
* If the instruction abort or the data abort due to
access to stage 1
* tran
Re: [RFC v2 6/8] tools/arm: Introduce force_assign_without_iommu option to xl.cfg
Hi Julien,
On Fri, Feb 18, 2022 at 10:17:33AM +, Julien Grall wrote:
>
>
> On 18/02/2022 09:16, Oleksii Moisieiev wrote:
> > Hi Julien,
>
> Hi Oleksii,
>
> > On Thu, Feb 17, 2022 at 03:20:36PM +, Julien Grall wrote:
> > > >xlu_cfg_get_defbool(config, "xend_suspend_evtchn_compat",
> > > > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > > > index 093bb4403f..f1f19bf711 100644
> > > > --- a/xen/common/domain.c
> > > > +++ b/xen/common/domain.c
> > > > @@ -512,7 +512,7 @@ static int sanitise_domain_config(struct
> > > > xen_domctl_createdomain *config)
> > > >if ( iommu )
> > > >{
> > > > -if ( config->iommu_opts & ~XEN_DOMCTL_IOMMU_no_sharept )
> > > > +if ( config->iommu_opts >> XEN_DOMCTL_IOMMU_MAX )
> > >
> > > XEN_DOMCTL_IOMMU_MAX will be defined as:
> > >
> > > (1U << _XEN_DOMCTL_IOMMU_force_iommu)
> > >
> > > This means the shift will do the wrong thing. However, AFAICT, this new
> > > option will only be supported by Arm and likely only for platform device
> > > for
> > > the time being.
> >
> > Thanks, I will fix that.
> >
> > >
> > > That said, I am not convinced this flag should be per-domain in Xen.
> > > Instead, I think it would be better to pass the flag via the device assign
> > > domctl.
> >
> > Do you mean that it's better to set this flag per device, not per
> > domain? > This will require setting this flag for each device which should
> > require either changing the dtdev format in dom.cfg or setting
> > xen,force-assign-without-iommu in partial device-tree.
> >
> > Both of those ways will complicate the configuration. As was mentioned
> > before, we don't want to make domain configuration more complicated.
> > What do you think about that?
>
> We have two interfaces here:
> 1) User -> tools
> 2) tools -> Xen
>
> We can chose different policy for each interface.
>
> For the tools -> Xen interface, I think this should be per device (similar
> to XEN_DOMCTL_DEV_RDM_RELAXED).
>
> For the User -> tools, I am open to discussion. One advantage with per
> device is the user explicitely vet each device. So it is harder to
> passthrough a device wrongly.
>
> But I agree this also complicates the interface. What do other thinks?
>
I see the following ways of User -> tools format:
a) Set force_assign_without_iommu = 1 in dom.cfg
b) Update dtdev format add force_iommu parameter, so dtdev will look
like this:
dtdev = [
"/soc/dma-controller@e670",
"/soc/gpio@e6055000,force_iommu",
...
]
c)...
Tools -> Xen possible ways:
d) Set force_assign_without_iommu to domain globally
e) Pass force_assign_without_iommu via device-assign domctl.
a) + d) is what we have in the patch series.
I think a) + e) can work for now so we will have an interface to make
force_assign_without_iommu per device in future.
What do you think about it?
> > >
> > > >return -EOPNOTSUPP;
> > > > @@ -542,7 +545,7 @@ int iommu_do_domctl(
> > > >#endif
> > > >#ifdef CONFIG_HAS_DEVICE_TREE
> > > > -if ( ret == -ENODEV )
> > > > +if ( ret == -ENOSYS )
> > >
> > > AFAICT, none of the code (including callee) before ret have been modified.
> > > So why are you modifying the check here?
> > >
> >
> > Because this check will fail if we have CONFIG_HAS_DEVICE_TREE define,
> > but do not have CONFIG_HAS_PCI and iommu_do_dt_domctl will not be
> > called.
>
> Below the implementation of iommu_do_domctl() on staging:
>
> int iommu_do_domctl(
> struct xen_domctl *domctl, struct domain *d,
> XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> {
> int ret = -ENODEV;
>
> if ( !is_iommu_enabled(d) )
> return -EOPNOTSUPP;
>
> #ifdef CONFIG_HAS_PCI
> ret = iommu_do_pci_domctl(domctl, d, u_domctl);
> #endif
>
> #ifdef CONFIG_HAS_DEVICE_TREE
> if ( ret == -ENODEV )
> ret = iommu_do_dt_domctl(domctl, d, u_domctl);
> #endif
>
> return ret;
> }
>
> 'ret' is initialized to -ENODEV. So for !CONFIG_HAS_PCI, then ret will not
> be changed. Therefore the current check is correct.
>
> AFAICT, your patch is setting 'ret' so I don't expect any change here.
>
> > Same thing if switch/case inside iommu_do_pci_domctl go to default and
> > return -ENOSYS. This part looked strange for me. But I will definitely
> > go through this part once again.
> We use the same sub-op to assign/deassign a PCI and "DT" device. So we are
> not interested in -ENOSYS but -ENODEV that would be returned by the checks:
>
> if ( domct->u.assign_device.dev != XEN_DOMCTL_DEV_PCI )
>
> At the moment, there are no sub-op specific to "DT" device. So it is not
> necessary for us to check -ENOSYS yet.
>
> I haven't looked at the rest of the series to see if we need it. But if we
> do, then I think the check should be extended in the patch that requires it.
>
Thank you for the comment. I will refactor this code.
Also I wanted to share with you some thoughts about using SMC client_id
field to pass agent_id to the SCM
[PATCH v2.1 8/7] x86/IOMMU: Use altcall, and __initconst_cf_clobber
Most IOMMU hooks are already altcall for performance reasons. Convert the
rest of them so we can harden all the hooks in Control Flow Integrity
configurations. This necessitates the use of iommu_{v,}call() in debug builds
too.
Move the root iommu_ops from __read_mostly to __ro_after_init now that the
latter exists. There is no need for a forward declaration of vtd_ops any
more, meaning that __initconst_cf_clobber can be used for VTD and AMD.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
---
xen/arch/x86/include/asm/iommu.h| 6 ++
xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +-
xen/drivers/passthrough/iommu.c | 7 ---
xen/drivers/passthrough/vtd/iommu.c | 3 +--
xen/drivers/passthrough/x86/iommu.c | 4 ++--
5 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/include/asm/iommu.h b/xen/arch/x86/include/asm/iommu.h
index 8a96ba1f097f..a87f6d416252 100644
--- a/xen/arch/x86/include/asm/iommu.h
+++ b/xen/arch/x86/include/asm/iommu.h
@@ -72,7 +72,6 @@ struct arch_iommu
extern struct iommu_ops iommu_ops;
-#ifdef NDEBUG
# include
# define iommu_call(ops, fn, args...) ({ \
(void)(ops); \
@@ -83,7 +82,6 @@ extern struct iommu_ops iommu_ops;
(void)(ops); \
alternative_vcall(iommu_ops.fn, ## args); \
})
-#endif
static inline const struct iommu_ops *iommu_get_ops(void)
{
@@ -106,7 +104,7 @@ int iommu_setup_hpet_msi(struct msi_desc *);
static inline int iommu_adjust_irq_affinities(void)
{
return iommu_ops.adjust_irq_affinities
- ? iommu_ops.adjust_irq_affinities()
+ ? iommu_call(iommu_ops, adjust_irq_affinities)
: 0;
}
@@ -122,7 +120,7 @@ int iommu_enable_x2apic(void);
static inline void iommu_disable_x2apic(void)
{
if ( x2apic_enabled && iommu_ops.disable_x2apic )
-iommu_ops.disable_x2apic();
+iommu_vcall(iommu_ops, disable_x2apic);
}
int iommu_identity_mapping(struct domain *d, p2m_access_t p2ma,
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index e57f555d00d1..4b59a4efe9b6 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -628,7 +628,7 @@ static void cf_check amd_dump_page_tables(struct domain *d)
hd->arch.amd.paging_mode, 0, 0);
}
-static const struct iommu_ops __initconstrel _iommu_ops = {
+static const struct iommu_ops __initconst_cf_clobber _iommu_ops = {
.init = amd_iommu_domain_init,
.hwdom_init = amd_iommu_hwdom_init,
.quarantine_init = amd_iommu_quarantine_init,
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index e220fea72c2f..c6b2c384d1dd 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -540,7 +540,7 @@ int __init iommu_setup(void)
int iommu_suspend()
{
if ( iommu_enabled )
-return iommu_get_ops()->suspend();
+return iommu_call(iommu_get_ops(), suspend);
return 0;
}
@@ -548,7 +548,7 @@ int iommu_suspend()
void iommu_resume()
{
if ( iommu_enabled )
-iommu_get_ops()->resume();
+iommu_vcall(iommu_get_ops(), resume);
}
int iommu_do_domctl(
@@ -578,7 +578,8 @@ void iommu_crash_shutdown(void)
return;
if ( iommu_enabled )
-iommu_get_ops()->crash_shutdown();
+iommu_vcall(iommu_get_ops(), crash_shutdown);
+
iommu_enabled = false;
#ifndef iommu_intremap
iommu_intremap = iommu_intremap_off;
diff --git a/xen/drivers/passthrough/vtd/iommu.c
b/xen/drivers/passthrough/vtd/iommu.c
index 56968a06a100..6a65ba1d8271 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -56,7 +56,6 @@ bool __read_mostly iommu_snoop = true;
static unsigned int __read_mostly nr_iommus;
-static struct iommu_ops vtd_ops;
static struct tasklet vtd_fault_tasklet;
static int cf_check setup_hwdom_device(u8 devfn, struct pci_dev *);
@@ -2794,7 +2793,7 @@ static int __init cf_check
intel_iommu_quarantine_init(struct domain *d)
return rc;
}
-static struct iommu_ops __initdata vtd_ops = {
+static const struct iommu_ops __initconst_cf_clobber vtd_ops = {
.init = intel_iommu_domain_init,
.hwdom_init = intel_iommu_hwdom_init,
.quarantine_init = intel_iommu_quarantine_init,
diff --git a/xen/drivers/passthrough/x86/iommu.c
b/xen/drivers/passthrough/x86/iommu.c
index ad5f44e13d98..17c0fe555dd0 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -27,7 +27,7 @@
#include
const struct iommu_init_ops *__initdata iommu_init_ops;
-struct iommu_ops __read_mostly iommu_ops;
+struct iommu_ops __ro_after_init iommu_ops;
bool __read_mostly iommu_non_coherent;
enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
@@ -129,7 +1
Re: [XEN v8 2/2] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler
On 21/02/2022 17:05, Ayan Kumar Halder wrote:
Hi Julien,
Hi,
On 13/02/2022 12:19, Julien Grall wrote:
}
void register_mmio_handler(struct domain *d,
diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
index 308650b400..3c0a935ccf 100644
--- a/xen/arch/arm/ioreq.c
+++ b/xen/arch/arm/ioreq.c
@@ -47,6 +47,7 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs
*regs,
struct vcpu *v, mmio_info_t *info)
{
struct vcpu_io *vio = &v->io;
+ struct dabt_instr instr = info->dabt_instr;
ioreq_t p = {
.type = IOREQ_TYPE_COPY,
.addr = info->gpa,
@@ -76,10 +77,8 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs
*regs,
if ( !s )
return IO_UNHANDLED;
- if ( !info->dabt.valid )
- return IO_ABORT;
-
For this one, I would switch to ASSERT(dabt.valid);
I see that try_fwd_ioserv() is invoked from try_handle_mmio() only.
Thus, if I follow your suggestion of adding a check for dabt.valid at
the beginning of try_handle_mmio(), then this ASSERT() is not required.
I agree that try_handle_mmio() is the only caller today. But we don't
know how this is going to be used tomorrow.
The goal of this ASSERT() is to catch those new users that would call it
wrongly.
[...]
... this will inject a data abort to the guest when we can't decode.
This is not what we want. We should check whether this is a P2M
translation fault or we need to map an MMIO region.
In pseudo-code, this would look like:
if ( !is_data || hsr.dabt.valid )
I think you mean if ( !is_data || !hsr.dabt.valid )
You are right.
The reason being if there is an instruction abort or a data abort (with
ISV == 0), then it should try to configure the page tables.
{
if ( check_p2m() )
return;
if ( !is_data )
goto inject_dabt;
decode_instruction();
if ( !dabt.invalid )
goto inject_dabt;
}
try_handle_mmio();
if ( instruction was not decoded )
check_p2m();
If the instruction was not decoded, then there is no need to configure
the page tables again. We have already done this before.
H... I think there are confusing about which sort of decoding I was
referring to. In this case, I mean if we didn't decode the instruction
manully, then it is not necessary to call check_p2m().
Do you agree with that?
So my understanding is as follows :-
/* Check that it is instruction abort or ISS is invalid. */
I have had a remark on this line before. Please have a look and address it.
if ( !is_data || !info.dabt.valid )
{
/*
* If the instruction was trapped due to access to stage 1
translation
* then Xen should try to resolve the page table entry for
the stage 1
* translation table with the assumption that the page
tables are
* present in the non MMIO region. If it is successful,
then it should
* ask the guest to retry the instruction.
*/
I agree that we want to skip the MMIO mapping when s1ptw == 1. However,
I am not sure this belongs to this patch because this is technically
already a bug.
if ( is_data && info.dabt.s1ptw )
{
info.dabt_instr.state = INSTR_RETRY;
/* The translation tables are assumed to be in non MMIO
region. */
is_data = false;
is_data is also used to decide which sort of abort we want to send to
the guest (see after inject_dabt). So I don't think we could force set
is_data here.
Instead, I would define a new local variable (maybe mmio_access_allowed)
that will be set for instruction abort or when s1ptw is 1.
}
/*
* Assumption :- Most of the times when we get a
translation fault
* and the ISS is invalid, the underlying cause is that the
page
* tables have not been set up correctly.
*/
I think this comment make more sense on top of "if !is_data ||
!info.dabt.valid".
if ( check_p2m(is_data, gpa) )
return;
/*
* If the instruction abort or the data abort due to access
to stage 1
* translation tables could not be resolved by setting the
appropriate
* bits in the translation table, then Xen should abort the
guest.
IHMO, "abort the guest" means we are going to crash the guest. However,
this not the case here. We are telling the guest that we couldn't handle
the data/instruction request. It is up to the guest to decide whether it
should panic or handle gracefully the error.
We should also avoid the term guest because it usually only refers to
any domain but dom0.
Therefore, I would reword it to something like "Xen will forward the
data/instruction abort to the domain".
*/
if ( !is_data || (info.dabt_instr.state == INSTR_RETRY) )
The second part looks
[ovmf test] 168185: all pass - PUSHED
flight 168185 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/168185/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf b24306f15daa2ff8510b06702114724b33895d3c baseline version: ovmf 8a576733162bb72afb4d1eb3012b0aef8d265018 Last test of basis 168131 2022-02-16 12:13:29 Z5 days Testing same since 168185 2022-02-21 15:43:05 Z0 days1 attempts People who touched revisions under test: Heng Luo jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 8a57673316..b24306f15d b24306f15daa2ff8510b06702114724b33895d3c -> xen-tested-master
Re: [XEN v8 2/2] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler
Hi Julien,
Apprecciate your continuous help on this. Most of your comments make
sense, but I need few clarifications as below :-
On 13/02/2022 12:19, Julien Grall wrote:
Hi,
On 12/02/2022 23:34, Ayan Kumar Halder wrote:
xen/arch/arm/arm32/traps.c | 7 +++
xen/arch/arm/arm64/traps.c | 47 +++
xen/arch/arm/decode.c | 1 +
xen/arch/arm/include/asm/domain.h | 4 ++
xen/arch/arm/include/asm/mmio.h | 15 -
xen/arch/arm/include/asm/traps.h | 2 +
xen/arch/arm/io.c | 98 ---
xen/arch/arm/ioreq.c | 7 ++-
xen/arch/arm/traps.c | 80 +
xen/arch/x86/include/asm/ioreq.h | 3 +
This change technically needs an ack from the x86 maintainers. And...
xen/include/xen/sched.h | 2 +
this one for anyone from THE REST (Stefano and I are part of it).
Please use scripts/add_maintainers.pl to automatically add the
relevant maintainers in CC.
11 files changed, 217 insertions(+), 49 deletions(-)
diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
index 9c9790a6d1..70c6238196 100644
--- a/xen/arch/arm/arm32/traps.c
+++ b/xen/arch/arm/arm32/traps.c
@@ -18,9 +18,11 @@
#include
#include
+#include
#include
+#include
#include
#include
@@ -82,6 +84,11 @@ void do_trap_data_abort(struct cpu_user_regs *regs)
do_unexpected_trap("Data Abort", regs);
}
+void post_increment_register(const struct instr_details *instr)
+{
+ domain_crash(current->domain);
Please add a comment explaning why this is resulting to a domain
crash. AFAICT, this is because this should not be reachable (yet) for
32-bit.
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/arm64/traps.c b/xen/arch/arm/arm64/traps.c
index 9113a15c7a..a6766689b3 100644
--- a/xen/arch/arm/arm64/traps.c
+++ b/xen/arch/arm/arm64/traps.c
@@ -23,6 +23,7 @@
#include
#include
+#include
The headers should ordered so are first, then ,
then . They should then be ordered alphabetically within
each of the category.
So, this new header should be included right after
[...]
diff --git a/xen/arch/arm/include/asm/mmio.h
b/xen/arch/arm/include/asm/mmio.h
index 3354d9c635..745130b7fe 100644
--- a/xen/arch/arm/include/asm/mmio.h
+++ b/xen/arch/arm/include/asm/mmio.h
@@ -26,12 +26,22 @@
#define MAX_IO_HANDLER 16
+enum instr_decode_state
+{
+ INSTR_ERROR, /* Error encountered while
decoding instr */
+ INSTR_VALID, /* ISS is valid, so no need to
decode */
+ INSTR_LDR_STR_POSTINDEXING, /* Instruction is decoded
successfully.
+ It is ldr/str post indexing */
Coding style: multiple-line comments for Xen should be:
/*
* ...
* ...
*/
In this case, I would simply move the comment on top.
[...]
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index a289d393f9..203466b869 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -95,57 +95,87 @@ static const struct mmio_handler
*find_mmio_handler(struct domain *d,
return handler;
}
+void try_decode_instruction(const struct cpu_user_regs *regs,
+ mmio_info_t *info)
+{
+ int rc;
+
+ /*
+ * Erratum 766422: Thumb store translation fault to Hypervisor may
+ * not have correct HSR Rt value.
+ */
+ if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
+ info->dabt.write )
+ {
+ rc = decode_instruction(regs, info);
+ if ( rc )
+ {
+ gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
+ info->dabt_instr.state = INSTR_ERROR;
+ return;
+ }
+ }
At the moment, the errata would only be handled when the ISS is valid.
Now, you are moving it before we know if it is valid. Can you explain
why?
[...]
enum io_state try_handle_mmio(struct cpu_user_regs *regs,
- const union hsr hsr,
- paddr_t gpa)
+ mmio_info_t *info)
{
struct vcpu *v = current;
const struct mmio_handler *handler = NULL;
- const struct hsr_dabt dabt = hsr.dabt;
- mmio_info_t info = {
- .gpa = gpa,
- .dabt = dabt
- };
+ int rc;
- ASSERT(hsr.ec == HSR_EC_DATA_ABORT_LOWER_EL);
+ ASSERT(info->dabt.ec == HSR_EC_DATA_ABORT_LOWER_EL);
- handler = find_mmio_handler(v->domain, info.gpa);
+ handler = find_mmio_handler(v->domain, info->gpa);
if ( !handler )
{
- int rc;
-
- rc = try_fwd_ioserv(regs, v, &info);
+ rc = try_fwd_ioserv(regs, v, info);
if ( rc == IO_HANDLED )
return handle_ioserv(regs, v);
return rc;
}
- /* All the instructions used on emulated MMIO region should be
valid */
- if ( !dabt.valid )
- return IO_ABORT;
-
AFAIU, the assumption i
Re: [PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit
Hi Michal, On 21/02/2022 10:59, Michal Orzel wrote: Following a discussion [1] it seems like that renaming work has been forgotten. This is in my todo list of clean-up I need to do for Xen. But I haven't yet had a chance to look at it. Thank you for taking a look! Perform renaming of psr_mode_is_32bit to regs_mode_is_32bit as the function no longer takes psr parameter. If we modify psr_mode_is_32bit(), then we should also modify psr_mode_is_user() because they have the same prototype and we should keep the naming consistent. [1] https://marc.info/?l=xen-devel&m=156457538423787&w=2 NIT: The first sentence and this link adds value for the review on the mailing list (we know where the request came from) but doesn't add any after the commit message (there are no extra information in them). So I would move this information after ---. This will get dropped on commit. Cheers, -- Julien Grall
Re: [PATCH] xen/include/public: add macro for invalid grant reference
On 21.02.22 16:31, Jan Beulich wrote: On 21.02.2022 16:05, Juergen Gross wrote: On 21.02.22 15:31, Jan Beulich wrote: On 21.02.2022 15:27, Juergen Gross wrote: On 21.02.22 15:18, Jan Beulich wrote: On 21.02.2022 13:42, Juergen Gross wrote: Providing a macro for an invalid grant reference would be beneficial for users, especially as some are using the wrong value "0" for that purpose. Signed-off-by: Juergen Gross Over the years I've been considering to add such to the public interface, perhaps even more than once. But I'm afraid it's not that easy. In principle 0x (which btw isn't necessarily ~0u) could I can change that to use 0x explicitly. be a valid ref. It is really internal agreement by users of the interface to set for themselves that they're not ever going to make a valid grant behind that reference. As the grant reference is an index into the grant table this would limit the size of the grant table to "only" UINT_MAX - 1. I don't think this will be ever a concern (other than an academical one). That wasn't my point. Limiting the table to one less entry is not a big deal indeed. But we have no reason to mandate which gref(s) to consider invalid. A guest could consider gref 0 the invalid one. With the gref being an index starting with 0 (gref 0 is valid, as it is used for the console ring page), the natural choice for an invalid value is the highest one being representable. A gref is of type uint32_t resulting in this value being 0x. While in theory a grant table could be that large, in practice this will never happen. The hypervisor doesn't care. Imo this simply is an aspect which is This isn't true. The hypervisor needs to allocate resources for being able to handle the highest possible gref value for a guest. For a v1 grant table this would mean 32GB of grant table size. Are you really concerned we will ever hit this limit? This isn't at the guest's choice, after all, as the max grant table size is limited by Xen. If we're not going to hit that limit, what's wrong with declaring the entire upper half of uint32_t space "invalid" for use a gref? If we won't ever go up to 32Gb, we quite certainly also won't ever reach 16Gb. Yes, you probably already guessed it, we can then repeat this process iteratively until we reach 4kb. This reasoning is nonsense, and you know it. not in need of pinning down in the ABI. Yet if it was pinned down like you do, then the hypervisor would need to make sure it refuses to act on this mandated invalid gref. This is an easy one. We could just refuse to have a grant table of that size. I can add this to the patch if you really think it is necessary. Since grant table size is measured in pages, you'd then have to refuse use of more than just that single gref. This would still not be an immediate problem, but demonstrates again that it's unclear where to draw such a boundary, if one is to be artificially drawn. It should be as high as possible. I wouldn't mind just refusing the last possible gref, but I don't think this is necessary. TBH, I think such completely theoretical concerns should not stand in the way of additions to the ABI making life easier for consumers. In case it wasn't clear - my concern isn't that sacrificing this one entry may cause a problem, or that we'd ever see grant tables grow this big (albeit for the latter: you never really know). Instead my concern is that it is conceptually wrong for us to (now) introduce such a value. I have understood that this is your concern. I continue to think that this concern is of purely academical nature. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v6 16/21] mips: Use do_kernel_power_off()
On Mon, Jan 31, 2022 at 02:37:13AM +0300, Dmitry Osipenko wrote:
> Kernel now supports chained power-off handlers. Use do_kernel_power_off()
> that invokes chained power-off handlers. It also invokes legacy
> pm_power_off() for now, which will be removed once all drivers will
> be converted to the new power-off API.
>
> Signed-off-by: Dmitry Osipenko
> ---
> arch/mips/kernel/reset.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/mips/kernel/reset.c b/arch/mips/kernel/reset.c
> index 6288780b779e..e7ce07b3e79b 100644
> --- a/arch/mips/kernel/reset.c
> +++ b/arch/mips/kernel/reset.c
> @@ -114,8 +114,7 @@ void machine_halt(void)
>
> void machine_power_off(void)
> {
> - if (pm_power_off)
> - pm_power_off();
> + do_kernel_power_off();
>
> #ifdef CONFIG_SMP
> preempt_disable();
> --
> 2.34.1
Ackey-by: Thomas Bogendoerfer
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.[ RFC1925, 2.3 ]
Re: [PATCH] xen/include/public: add macro for invalid grant reference
On 21.02.2022 16:05, Juergen Gross wrote: > On 21.02.22 15:31, Jan Beulich wrote: >> On 21.02.2022 15:27, Juergen Gross wrote: >>> On 21.02.22 15:18, Jan Beulich wrote: On 21.02.2022 13:42, Juergen Gross wrote: > Providing a macro for an invalid grant reference would be beneficial > for users, especially as some are using the wrong value "0" for that > purpose. > > Signed-off-by: Juergen Gross Over the years I've been considering to add such to the public interface, perhaps even more than once. But I'm afraid it's not that easy. In principle 0x (which btw isn't necessarily ~0u) could >>> >>> I can change that to use 0x explicitly. >>> be a valid ref. It is really internal agreement by users of the interface to set for themselves that they're not ever going to make a valid grant behind that reference. >>> >>> As the grant reference is an index into the grant table this would >>> limit the size of the grant table to "only" UINT_MAX - 1. I don't >>> think this will be ever a concern (other than an academical one). >> >> That wasn't my point. Limiting the table to one less entry is not a >> big deal indeed. But we have no reason to mandate which gref(s) to >> consider invalid. A guest could consider gref 0 the invalid one. > > With the gref being an index starting with 0 (gref 0 is valid, as it is > used for the console ring page), the natural choice for an invalid > value is the highest one being representable. A gref is of type uint32_t > resulting in this value being 0x. > > While in theory a grant table could be that large, in practice this > will never happen. > >> The hypervisor doesn't care. Imo this simply is an aspect which is > > This isn't true. The hypervisor needs to allocate resources for being > able to handle the highest possible gref value for a guest. For a v1 > grant table this would mean 32GB of grant table size. Are you really > concerned we will ever hit this limit? This isn't at the guest's > choice, after all, as the max grant table size is limited by Xen. If we're not going to hit that limit, what's wrong with declaring the entire upper half of uint32_t space "invalid" for use a gref? If we won't ever go up to 32Gb, we quite certainly also won't ever reach 16Gb. Yes, you probably already guessed it, we can then repeat this process iteratively until we reach 4kb. >> not in need of pinning down in the ABI. Yet if it was pinned down >> like you do, then the hypervisor would need to make sure it refuses >> to act on this mandated invalid gref. > > This is an easy one. We could just refuse to have a grant table of > that size. I can add this to the patch if you really think it is > necessary. Since grant table size is measured in pages, you'd then have to refuse use of more than just that single gref. This would still not be an immediate problem, but demonstrates again that it's unclear where to draw such a boundary, if one is to be artificially drawn. > TBH, I think such completely theoretical concerns should not stand > in the way of additions to the ABI making life easier for consumers. In case it wasn't clear - my concern isn't that sacrificing this one entry may cause a problem, or that we'd ever see grant tables grow this big (albeit for the latter: you never really know). Instead my concern is that it is conceptually wrong for us to (now) introduce such a value. Jan
Re: [PATCH] xen/include/public: add macro for invalid grant reference
On 21.02.22 15:31, Jan Beulich wrote: On 21.02.2022 15:27, Juergen Gross wrote: On 21.02.22 15:18, Jan Beulich wrote: On 21.02.2022 13:42, Juergen Gross wrote: Providing a macro for an invalid grant reference would be beneficial for users, especially as some are using the wrong value "0" for that purpose. Signed-off-by: Juergen Gross Over the years I've been considering to add such to the public interface, perhaps even more than once. But I'm afraid it's not that easy. In principle 0x (which btw isn't necessarily ~0u) could I can change that to use 0x explicitly. be a valid ref. It is really internal agreement by users of the interface to set for themselves that they're not ever going to make a valid grant behind that reference. As the grant reference is an index into the grant table this would limit the size of the grant table to "only" UINT_MAX - 1. I don't think this will be ever a concern (other than an academical one). That wasn't my point. Limiting the table to one less entry is not a big deal indeed. But we have no reason to mandate which gref(s) to consider invalid. A guest could consider gref 0 the invalid one. With the gref being an index starting with 0 (gref 0 is valid, as it is used for the console ring page), the natural choice for an invalid value is the highest one being representable. A gref is of type uint32_t resulting in this value being 0x. While in theory a grant table could be that large, in practice this will never happen. The hypervisor doesn't care. Imo this simply is an aspect which is This isn't true. The hypervisor needs to allocate resources for being able to handle the highest possible gref value for a guest. For a v1 grant table this would mean 32GB of grant table size. Are you really concerned we will ever hit this limit? This isn't at the guest's choice, after all, as the max grant table size is limited by Xen. not in need of pinning down in the ABI. Yet if it was pinned down like you do, then the hypervisor would need to make sure it refuses to act on this mandated invalid gref. This is an easy one. We could just refuse to have a grant table of that size. I can add this to the patch if you really think it is necessary. TBH, I think such completely theoretical concerns should not stand in the way of additions to the ABI making life easier for consumers. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] xen/include/public: add macro for invalid grant reference
On 21.02.2022 15:27, Juergen Gross wrote: > On 21.02.22 15:18, Jan Beulich wrote: >> On 21.02.2022 13:42, Juergen Gross wrote: >>> Providing a macro for an invalid grant reference would be beneficial >>> for users, especially as some are using the wrong value "0" for that >>> purpose. >>> >>> Signed-off-by: Juergen Gross >> >> Over the years I've been considering to add such to the public >> interface, perhaps even more than once. But I'm afraid it's not that >> easy. In principle 0x (which btw isn't necessarily ~0u) could > > I can change that to use 0x explicitly. > >> be a valid ref. It is really internal agreement by users of the >> interface to set for themselves that they're not ever going to make >> a valid grant behind that reference. > > As the grant reference is an index into the grant table this would > limit the size of the grant table to "only" UINT_MAX - 1. I don't > think this will be ever a concern (other than an academical one). That wasn't my point. Limiting the table to one less entry is not a big deal indeed. But we have no reason to mandate which gref(s) to consider invalid. A guest could consider gref 0 the invalid one. The hypervisor doesn't care. Imo this simply is an aspect which is not in need of pinning down in the ABI. Yet if it was pinned down like you do, then the hypervisor would need to make sure it refuses to act on this mandated invalid gref. Jan
Re: [PATCH] xen/include/public: add macro for invalid grant reference
On 21.02.22 15:18, Jan Beulich wrote: On 21.02.2022 13:42, Juergen Gross wrote: Providing a macro for an invalid grant reference would be beneficial for users, especially as some are using the wrong value "0" for that purpose. Signed-off-by: Juergen Gross Over the years I've been considering to add such to the public interface, perhaps even more than once. But I'm afraid it's not that easy. In principle 0x (which btw isn't necessarily ~0u) could I can change that to use 0x explicitly. be a valid ref. It is really internal agreement by users of the interface to set for themselves that they're not ever going to make a valid grant behind that reference. As the grant reference is an index into the grant table this would limit the size of the grant table to "only" UINT_MAX - 1. I don't think this will be ever a concern (other than an academical one). Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 2/3] xen: Rename asprintf() to xasprintf()
On 21/02/2022 12:23, Roger Pau Monne wrote: > On Mon, Feb 21, 2022 at 11:28:39AM +, Andrew Cooper wrote: >> On 21/02/2022 11:21, Roger Pau Monné wrote: >>> On Mon, Feb 21, 2022 at 10:02:53AM +, Andrew Cooper wrote: Coverity reports that there is a memory leak in ioreq_server_alloc_rangesets(). This would be true if Xen's implementation of asprintf() had glibc's return semantics, but it doesn't. Rename to xasprintf() to reduce confusion for Coverity and other developers. >>> It would seem more natural to me to rename to asprintk. >> Why? This infrastructure doesn't emit the string to any console. > Right, but the f in printf is for print formatted, not for where the > output is supposed to go. So printk is the outlier and should instead > be kprintf? > > I can buy into using xasprintf (also because that's what Linux does > with kasprintf), but I don't think it's so obvious given the precedent > of having printk instead of printf. The naming isn't ideal, but this is Xen's local version of the thing called asprintf() in userspace. Naming it anything other than xasprintf() is going to be even more confusing for people than this mess already is... >>> I assume >>> there's no way for Coverity to prevent overrides with builtin models? >>> >>> I've been searching, but there doesn't seem to be any option to >>> prevent overrides by builtin models? >> No, and we absolutely wouldn't want to skip the model even if we could, >> because that would break asprintf() analysis for userspace. > Well, we could maybe find a way to only enable the flag for hypervisor > code build, but anyway, it's pointless to discus if there's no flag in > the first place. > > Coverity could be clever enough to check if there's an implementation > provided for those, instead of unconditionally override with a > model. There is no way to disable the model for asprintf(). ~Andrew
Re: [PATCH] xen/include/public: add macro for invalid grant reference
On 21.02.2022 13:42, Juergen Gross wrote: > Providing a macro for an invalid grant reference would be beneficial > for users, especially as some are using the wrong value "0" for that > purpose. > > Signed-off-by: Juergen Gross Over the years I've been considering to add such to the public interface, perhaps even more than once. But I'm afraid it's not that easy. In principle 0x (which btw isn't necessarily ~0u) could be a valid ref. It is really internal agreement by users of the interface to set for themselves that they're not ever going to make a valid grant behind that reference. Jan
Re: [PATCH 2/3] xen: Rename asprintf() to xasprintf()
On 21.02.2022 11:02, Andrew Cooper wrote:
> Coverity reports that there is a memory leak in
> ioreq_server_alloc_rangesets(). This would be true if Xen's implementation of
> asprintf() had glibc's return semantics, but it doesn't.
>
> Rename to xasprintf() to reduce confusion for Coverity and other developers.
>
> While at it, fix style issues. Rearrange ioreq_server_alloc_rangesets() to
> use a tabulated switch statement, and not to have a trailing space in the
> rangeset name for an unknown range type.
>
> Coverity-ID: 1472735
> Coverity-ID: 1500265
> Fixes: 780e918a2e54 ("add an implentation of asprintf() for xen")
> Signed-off-by: Andrew Cooper
Reviewed-by: Jan Beulich
[PATCH] xen/include/public: add macro for invalid grant reference
Providing a macro for an invalid grant reference would be beneficial for users, especially as some are using the wrong value "0" for that purpose. Signed-off-by: Juergen Gross --- xen/include/public/grant_table.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h index 7fbd1c6d10..af00aacfd3 100644 --- a/xen/include/public/grant_table.h +++ b/xen/include/public/grant_table.h @@ -113,6 +113,8 @@ */ typedef uint32_t grant_ref_t; +#define XEN_GRANT_REF_INVALID ~0U + /* * A grant table comprises a packed array of grant entries in one or more * page frames shared between Xen and a guest. -- 2.34.1
Re: [PATCH 2/3] xen: Rename asprintf() to xasprintf()
On Mon, Feb 21, 2022 at 11:28:39AM +, Andrew Cooper wrote: > On 21/02/2022 11:21, Roger Pau Monné wrote: > > On Mon, Feb 21, 2022 at 10:02:53AM +, Andrew Cooper wrote: > >> Coverity reports that there is a memory leak in > >> ioreq_server_alloc_rangesets(). This would be true if Xen's > >> implementation of > >> asprintf() had glibc's return semantics, but it doesn't. > >> > >> Rename to xasprintf() to reduce confusion for Coverity and other > >> developers. > > It would seem more natural to me to rename to asprintk. > > Why? This infrastructure doesn't emit the string to any console. Right, but the f in printf is for print formatted, not for where the output is supposed to go. So printk is the outlier and should instead be kprintf? I can buy into using xasprintf (also because that's what Linux does with kasprintf), but I don't think it's so obvious given the precedent of having printk instead of printf. > > I assume > > there's no way for Coverity to prevent overrides with builtin models? > > > > I've been searching, but there doesn't seem to be any option to > > prevent overrides by builtin models? > > No, and we absolutely wouldn't want to skip the model even if we could, > because that would break asprintf() analysis for userspace. Well, we could maybe find a way to only enable the flag for hypervisor code build, but anyway, it's pointless to discus if there's no flag in the first place. Coverity could be clever enough to check if there's an implementation provided for those, instead of unconditionally override with a model. Thanks, Roger.
[linux-linus test] 168183: tolerable FAIL - PUSHED
flight 168183 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/168183/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 168179 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 168179 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168179 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168179 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 168179 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168179 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 168179 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 168179 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass version targeted for testing: linuxcfb92440ee71adcc2105b0890bb01ac3cddb8507 baseline version: linux7f25f0412c9e2be6811e8aedbd10ef795fff85f2 Last test of basis 168179 2022-02-20 19:11:13 Z0 days Testing same since 168183 2022-02-21 02:00:50 Z0 days1 attempts People who touched revisions under test: Andy Lutomirski Benjamin Tissoires Bjorn Andersson Borislav Petkov Cheng Jui Wang Christophe JAILLET Dave Hansen Dietmar Eggemann Dmitry Torokhov Eliav Farber Eric Anholt Florian Fainelli James Smart Jarkko Nikula Jiasheng Jiang Jinyoung Choi Jiri Kosina José Expósito Linus Torvalds Linus Walleij Martin K. Petersen Miaoqia
[libvirt test] 168184: regressions - FAIL
flight 168184 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/168184/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 151777 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a version targeted for testing: libvirt 454b927d1e33a1fe9dca535db2c97300fdae62cc baseline version: libvirt 2c846fa6bcc11929c9fb857a22430fb9945654ad Last test of basis 151777 2020-07-10 04:19:19 Z 591 days Failing since151818 2020-07-11 04:18:52 Z 590 days 572 attempts Testing same since 168171 2022-02-19 04:18:57 Z2 days3 attempts People who touched revisions under test: Adolfo Jayme Barrientos Aleksandr Alekseev Aleksei Zakharov Andika Triwidada Andrea Bolognani Ani Sinha Balázs Meskó Barrett Schonefeld Bastian Germann Bastien Orivel BiaoXiang Ye Bihong Yu Binfeng Wu Bjoern Walk Boris Fiuczynski Brad Laue Brian Turek Bruno Haible Chris Mayo Christian Borntraeger Christian Ehrhardt Christian Kirbach Christian Schoenebeck Christophe Fergeau Cole Robinson Collin Walling Cornelia Huck Cédric Bosdonnat Côme Borsoi Daniel Henrique Barboza Daniel Letai Daniel P. Berrange Daniel P. Berrangé Didik Supriadi dinglimin Divya Garg Dmitrii Shcherbakov Dmytro Linkin Eiichi Tsukata Emilio Herrera Eric Farman Erik Skultety Fabian Affolter Fabian Freyer Fabiano Fidêncio Fangge Jin Farhan Ali Fedora Weblate Translation Franck Ridel Gavi Teitz gongwei Guoyi Tu Göran Uddeborg Halil Pasic Han Han Hao Wang Hela Basa Helmut Grohne Hiroki Narukawa Hyman Huang(黄勇) Ian Wienand Ioanna Alifieraki Ivan Teterevkov Jakob Meng Jamie Strandboge Jamie Strandboge Jan Kuparinen jason lee Jean-Baptiste Holcroft Jia Zhou Jianan Gao Jim Fehlig Jin Yan Jing Qi Jinsheng Zhang Jiri Denemark Joachim Falk John Ferlan Jonathan Watt Jonathon Jongsma Julio Faracco Justin Gatzen Ján Tomko Kashyap Chamarthy Kevin Locke Koichi Murase Kristina Hanicova Laine Stump Laszlo Ersek Lee Yarwood Lei Yang Liao Pingfang Lin Ma Lin Ma Lin Ma Liu Yiding Lubomir Rintel Luke Yue Luyao Zhong Marc Hartmayer Marc-André Lureau Marek Marczykowski-Górecki Markus Schade Martin Kletzander Masayoshi Mizuma Matej Cepl Matt Coleman Matt Coleman Mauro Matteo Cascella Meina Li Michal Privoznik Michał Smyk Milo Casagrande Moshe Levi Muha Aliss Nathan Neal Gompa Nick Chevsky Nick Shyrokovskiy Nickys Music Group Nico Pache Nicolas Lécureuil Nicolas Lécureuil Nikolay Shirokovskiy Olaf Hering Olesya Gerasimenko Or Ozeri Orion Poplawski Pany Patrick Magauran Paulo de Rezende Pinatti Pavel Hrdina Peng Liang Peter Krempa Pino Toscano Pino Toscano Piotr Drąg Prathamesh Chavan Praveen K Paladugu Richard W.M. Jones Ricky Tigg Robin Lee Rohit Kumar Roman Bogorodskiy Roman Bolshakov Ryan Gahagan Ryan Schmidt Sam Hartman Scott Shambarger Sebastian Mitterle SeongHyun Jo Shalini Chellathurai Saroja Shaojun Yang
Re: [PATCH v2] CI: Coverity tweaks
On 21/02/2022 11:22, Roger Pau Monné wrote: > On Mon, Feb 21, 2022 at 11:14:54AM +, Andrew Cooper wrote: >> * Use workflow_dispatch to allow manual creation of the job. > I guess such manual creation requires some kind of superpower > credentials on the github repo? I'd expect its open to project members. >> * Use parallel builds; the workers have two vCPUs. Also, use the build-* >>targets rather than the ones which expand to dist-*. >> * Shrink the dependency list further. build-essential covers make and gcc, >>while bridge-utils and iproute2 are runtime dependencies not build >>dependencies. Alter bzip2 to libbz2-dev. >> >> Signed-off-by: Andrew Cooper > Reviewed-by: Roger Pau Monné Thanks, ~Andrew
Re: [PATCH 2/3] xen: Rename asprintf() to xasprintf()
On 21/02/2022 11:21, Roger Pau Monné wrote: > On Mon, Feb 21, 2022 at 10:02:53AM +, Andrew Cooper wrote: >> Coverity reports that there is a memory leak in >> ioreq_server_alloc_rangesets(). This would be true if Xen's implementation >> of >> asprintf() had glibc's return semantics, but it doesn't. >> >> Rename to xasprintf() to reduce confusion for Coverity and other developers. > It would seem more natural to me to rename to asprintk. Why? This infrastructure doesn't emit the string to any console. > I assume > there's no way for Coverity to prevent overrides with builtin models? > > I've been searching, but there doesn't seem to be any option to > prevent overrides by builtin models? No, and we absolutely wouldn't want to skip the model even if we could, because that would break asprintf() analysis for userspace. ~Andrew
Re: [PATCH v2] CI: Coverity tweaks
On Mon, Feb 21, 2022 at 11:14:54AM +, Andrew Cooper wrote: > * Use workflow_dispatch to allow manual creation of the job. I guess such manual creation requires some kind of superpower credentials on the github repo? > * Use parallel builds; the workers have two vCPUs. Also, use the build-* >targets rather than the ones which expand to dist-*. > * Shrink the dependency list further. build-essential covers make and gcc, >while bridge-utils and iproute2 are runtime dependencies not build >dependencies. Alter bzip2 to libbz2-dev. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Thanks, Roger.
Re: [PATCH 2/3] xen: Rename asprintf() to xasprintf()
On Mon, Feb 21, 2022 at 10:02:53AM +, Andrew Cooper wrote: > Coverity reports that there is a memory leak in > ioreq_server_alloc_rangesets(). This would be true if Xen's implementation of > asprintf() had glibc's return semantics, but it doesn't. > > Rename to xasprintf() to reduce confusion for Coverity and other developers. It would seem more natural to me to rename to asprintk. I assume there's no way for Coverity to prevent overrides with builtin models? I've been searching, but there doesn't seem to be any option to prevent overrides by builtin models? Thanks, Roger.
Re: [PATCH] coverity: disable flight from crontab
On 21/02/2022 09:48, Roger Pau Monne wrote: > We are currently doing the Coverity Scans using a github workflow. > > Signed-off-by: Roger Pau Monné FWIW, Acked-by: Andrew Cooper
[PATCH v2] CI: Coverity tweaks
* Use workflow_dispatch to allow manual creation of the job.
* Use parallel builds; the workers have two vCPUs. Also, use the build-*
targets rather than the ones which expand to dist-*.
* Shrink the dependency list further. build-essential covers make and gcc,
while bridge-utils and iproute2 are runtime dependencies not build
dependencies. Alter bzip2 to libbz2-dev.
Signed-off-by: Andrew Cooper
---
CC: Roger Pau Monné
v2:
* Merge with existing command:
---
.github/workflows/coverity.yml | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 9d04b56fd31d..427fb86f947f 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -2,6 +2,7 @@ name: Coverity Scan
# We only want to test official release code, not every pull request.
on:
+ workflow_dispatch:
schedule:
- cron: '18 9 * * WED,SUN' # Bi-weekly at 9:18 UTC
@@ -11,11 +12,11 @@ jobs:
steps:
- name: Install build dependencies
run: |
-sudo apt-get install -y wget git gawk bridge-utils \
- iproute2 bzip2 build-essential \
- make gcc zlib1g-dev libncurses5-dev iasl \
- libbz2-dev e2fslibs-dev git-core uuid-dev ocaml \
- ocaml-findlib xz-utils libyajl-dev \
+sudo apt-get install -y wget git gawk \
+ libbz2-dev build-essential \
+ zlib1g-dev libncurses5-dev iasl \
+ libbz2-dev e2fslibs-dev uuid-dev ocaml \
+ ocaml-findlib libyajl-dev \
autoconf libtool liblzma-dev \
python3-dev golang python-dev libsystemd-dev
@@ -31,11 +32,11 @@ jobs:
- name: Pre build stuff
run: |
-make mini-os-dir
+make -j`nproc` mini-os-dir
- uses: vapier/coverity-scan-action@v1
with:
-command: make xen tools && make -C extras/mini-os/
+command: make -j`nproc` build-xen build-tools && make -j`nproc` -C
extras/mini-os/
project: XenProject
email: ${{ secrets.COVERITY_SCAN_EMAIL }}
token: ${{ secrets.COVERITY_SCAN_TOKEN }}
--
2.11.0
[PATCH] xen/arm: Rename psr_mode_is_32bit to regs_mode_is_32bit
Following a discussion [1] it seems like that renaming work has
been forgotten. Perform renaming of psr_mode_is_32bit to
regs_mode_is_32bit as the function no longer takes psr parameter.
[1] https://marc.info/?l=xen-devel&m=156457538423787&w=2
Signed-off-by: Michal Orzel
---
xen/arch/arm/include/asm/regs.h | 2 +-
xen/arch/arm/traps.c| 30 +++---
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/xen/arch/arm/include/asm/regs.h b/xen/arch/arm/include/asm/regs.h
index ec091a28a2..04e821138a 100644
--- a/xen/arch/arm/include/asm/regs.h
+++ b/xen/arch/arm/include/asm/regs.h
@@ -13,7 +13,7 @@
#define psr_mode(psr,m) (((psr) & PSR_MODE_MASK) == m)
-static inline bool psr_mode_is_32bit(const struct cpu_user_regs *regs)
+static inline bool regs_mode_is_32bit(const struct cpu_user_regs *regs)
{
#ifdef CONFIG_ARM_32
return true;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9339d12f58..0db8e42d65 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -896,7 +896,7 @@ static void _show_registers(const struct cpu_user_regs
*regs,
if ( guest_mode )
{
-if ( psr_mode_is_32bit(regs) )
+if ( regs_mode_is_32bit(regs) )
show_registers_32(regs, ctxt, guest_mode, v);
#ifdef CONFIG_ARM_64
else
@@ -1631,7 +1631,7 @@ int check_conditional_instr(struct cpu_user_regs *regs,
const union hsr hsr)
{
unsigned long it;
-BUG_ON( !psr_mode_is_32bit(regs) || !(cpsr & PSR_THUMB) );
+BUG_ON( !regs_mode_is_32bit(regs) || !(cpsr & PSR_THUMB) );
it = ( (cpsr >> (10-2)) & 0xfc) | ((cpsr >> 25) & 0x3 );
@@ -1656,7 +1656,7 @@ int check_conditional_instr(struct cpu_user_regs *regs,
const union hsr hsr)
void advance_pc(struct cpu_user_regs *regs, const union hsr hsr)
{
register_t itbits, cond, cpsr = regs->cpsr;
-bool is_thumb = psr_mode_is_32bit(regs) && (cpsr & PSR_THUMB);
+bool is_thumb = regs_mode_is_32bit(regs) && (cpsr & PSR_THUMB);
if ( is_thumb && (cpsr & PSR_IT_MASK) )
{
@@ -2098,37 +2098,37 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
advance_pc(regs, hsr);
break;
case HSR_EC_CP15_32:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_cp15_32);
do_cp15_32(regs, hsr);
break;
case HSR_EC_CP15_64:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_cp15_64);
do_cp15_64(regs, hsr);
break;
case HSR_EC_CP14_32:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_cp14_32);
do_cp14_32(regs, hsr);
break;
case HSR_EC_CP14_64:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_cp14_64);
do_cp14_64(regs, hsr);
break;
case HSR_EC_CP14_DBG:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_cp14_dbg);
do_cp14_dbg(regs, hsr);
break;
case HSR_EC_CP10:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_cp10);
do_cp10(regs, hsr);
break;
case HSR_EC_CP:
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_cp);
do_cp(regs, hsr);
break;
@@ -2139,7 +2139,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
* ARMv7 (DDI 0406C.b): B1.14.8
* ARMv8 (DDI 0487A.d): D1-1501 Table D1-44
*/
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_smc32);
do_trap_smc(regs, hsr);
break;
@@ -2147,7 +2147,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
{
register_t nr;
-GUEST_BUG_ON(!psr_mode_is_32bit(regs));
+GUEST_BUG_ON(!regs_mode_is_32bit(regs));
perfc_incr(trap_hvc32);
#ifndef NDEBUG
if ( (hsr.iss & 0xff00) == 0xff00 )
@@ -2162,7 +2162,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
}
#ifdef CONFIG_ARM_64
case HSR_EC_HVC64:
-GUEST_BUG_ON(psr_mode_is_32bit(regs));
+GUEST_BUG_ON(regs_mode_is_32bit(regs));
perfc_incr(trap_hvc64);
#ifndef NDEBUG
if ( (hsr.iss & 0xff00) == 0xff00 )
@@ -2178,12 +2178,12 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
*
* ARMv8 (DDI 0487A.d): D1-1501 Table D1-44
*/
-GUEST_BUG_ON(psr_mode_is_32bit(regs));
+GUEST_BUG_ON(regs_mode_is_32bit(regs));
perfc_incr(trap_smc64);
do_trap_smc(regs, hsr);
break;
case HSR_EC_SYSREG:
-GUES
Re: [PATCH 3/3] CI: Coverity tweaks
On 21/02/2022 10:37, Roger Pau Monné wrote:
> On Mon, Feb 21, 2022 at 10:02:54AM +, Andrew Cooper wrote:
>> * Use workflow_dispatch to allow manual creation of the job.
>> * Use parallel builds. The workers have two vCPUs.
>> * Shrink the dependency list further. build-essential covers make and gcc,
>>while bridge-utils and iproute2 are runtime dependencies not build
>>dependencies. Alter bzip2 to libbz2-dev.
>>
>> Signed-off-by: Andrew Cooper
>> ---
>> CC: Roger Pau Monné
>> ---
>> .github/workflows/coverity.yml | 14 --
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
>> index 9d04b56fd31d..6e7b81e74f72 100644
>> --- a/.github/workflows/coverity.yml
>> +++ b/.github/workflows/coverity.yml
>> @@ -2,6 +2,7 @@ name: Coverity Scan
>>
>> # We only want to test official release code, not every pull request.
>> on:
>> + workflow_dispatch:
>>schedule:
>> - cron: '18 9 * * WED,SUN' # Bi-weekly at 9:18 UTC
>>
>> @@ -11,11 +12,11 @@ jobs:
>> steps:
>> - name: Install build dependencies
>>run: |
>> -sudo apt-get install -y wget git gawk bridge-utils \
>> - iproute2 bzip2 build-essential \
>> - make gcc zlib1g-dev libncurses5-dev iasl \
>> - libbz2-dev e2fslibs-dev git-core uuid-dev ocaml \
>> - ocaml-findlib xz-utils libyajl-dev \
>> +sudo apt-get install -y wget git gawk \
>> + libbz2-dev build-essential \
>> + zlib1g-dev libncurses5-dev iasl \
>> + libbz2-dev e2fslibs-dev uuid-dev ocaml \
>> + ocaml-findlib libyajl-dev \
>>autoconf libtool liblzma-dev \
>>python3-dev golang python-dev libsystemd-dev
>>
>> @@ -31,7 +32,7 @@ jobs:
>>
>> - name: Pre build stuff
>>run: |
>> -make mini-os-dir
>> +make -j`nproc` mini-os-dir
>>
>> - uses: vapier/coverity-scan-action@v1
>>with:
>> @@ -39,3 +40,4 @@ jobs:
>> project: XenProject
>> email: ${{ secrets.COVERITY_SCAN_EMAIL }}
>> token: ${{ secrets.COVERITY_SCAN_TOKEN }}
>> +command: make -j`nproc` build
> There's already a 'command:' parameter set just before 'project:'.
Oh, so there is.
> Are
> we OK with using plain build?
>
> If so we would have to disable docs build and stubdom? We don't want
> to analyze all the newlib &c that's build as part of stubdoms?
The problem I was trying to work around there was that xen&tools turn
into *-install so we also spend time shuffling binaries around the build
environment.
What we actually want is:
make -j`nproc` build-xen build-tools && make -j`nproc` -C extras/mini-os/
~Andrew
[PATCH v3 14/19] xen/arm: add Persistent Map (PMAP) infrastructure
From: Wei Liu
The basic idea is like Persistent Kernel Map (PKMAP) in Linux. We
pre-populate all the relevant page tables before the system is fully
set up.
We will need it on Arm in order to rework the arm64 version of
xenheap_setup_mappings() as we may need to use pages allocated from
the boot allocator before they are effectively mapped.
This infrastructure is not lock-protected therefore can only be used
before smpboot. After smpboot, map_domain_page() has to be used.
This is based on the x86 version [1] that was originally implemented
by Wei Liu.
The PMAP infrastructure is implemented in common code with some
arch helpers to set/clear the page-table entries and convertion
between a fixmap slot to a virtual address...
As mfn_to_xen_entry() now needs to be exported, take the opportunity
to swich the parameter attr from unsigned to unsigned int.
[1]
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
[julien: Adapted for Arm]
Signed-off-by: Julien Grall
---
Changes in v3:
- s/BITS_PER_LONG/BITS_PER_BYTE/
- Move pmap to common code
Changes in v2:
- New patch
Cc: Jan Beulich
Cc: Wei Liu
Cc: Andrew Cooper
Cc: Roger Pau Monné
---
xen/arch/arm/Kconfig | 1 +
xen/arch/arm/include/asm/fixmap.h | 17 +++
xen/arch/arm/include/asm/lpae.h | 8
xen/arch/arm/include/asm/pmap.h | 33 +
xen/arch/arm/mm.c | 7 +--
xen/common/Kconfig| 3 ++
xen/common/Makefile | 1 +
xen/common/pmap.c | 79 +++
xen/include/xen/pmap.h| 16 +++
9 files changed, 159 insertions(+), 6 deletions(-)
create mode 100644 xen/arch/arm/include/asm/pmap.h
create mode 100644 xen/common/pmap.c
create mode 100644 xen/include/xen/pmap.h
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index ecfa6822e4d3..a89a67802aa9 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -14,6 +14,7 @@ config ARM
select HAS_DEVICE_TREE
select HAS_PASSTHROUGH
select HAS_PDX
+ select HAS_PMAP
select IOMMU_FORCE_PT_SHARE
config ARCH_DEFCONFIG
diff --git a/xen/arch/arm/include/asm/fixmap.h
b/xen/arch/arm/include/asm/fixmap.h
index 1cee51e52ab9..c46a15e59de4 100644
--- a/xen/arch/arm/include/asm/fixmap.h
+++ b/xen/arch/arm/include/asm/fixmap.h
@@ -5,12 +5,20 @@
#define __ASM_FIXMAP_H
#include
+#include
/* Fixmap slots */
#define FIXMAP_CONSOLE 0 /* The primary UART */
#define FIXMAP_MISC 1 /* Ephemeral mappings of hardware */
#define FIXMAP_ACPI_BEGIN 2 /* Start mappings of ACPI tables */
#define FIXMAP_ACPI_END(FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1) /*
End mappings of ACPI tables */
+#define FIXMAP_PMAP_BEGIN (FIXMAP_ACPI_END + 1) /* Start of PMAP */
+#define FIXMAP_PMAP_END (FIXMAP_PMAP_BEGIN + NUM_FIX_PMAP - 1) /* End of PMAP
*/
+
+#define FIXMAP_LAST FIXMAP_PMAP_END
+
+#define FIXADDR_START FIXMAP_ADDR(0)
+#define FIXADDR_TOP FIXMAP_ADDR(FIXMAP_LAST)
#ifndef __ASSEMBLY__
@@ -19,6 +27,15 @@ extern void set_fixmap(unsigned map, mfn_t mfn, unsigned
attributes);
/* Remove a mapping from a fixmap entry */
extern void clear_fixmap(unsigned map);
+#define fix_to_virt(slot) ((void *)FIXMAP_ADDR(slot))
+
+static inline unsigned int virt_to_fix(vaddr_t vaddr)
+{
+BUG_ON(vaddr >= FIXADDR_TOP || vaddr < FIXADDR_START);
+
+return ((vaddr - FIXADDR_START) >> PAGE_SHIFT);
+}
+
#endif /* __ASSEMBLY__ */
#endif /* __ASM_FIXMAP_H */
diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
index 8cf932b5c947..6099037da1c0 100644
--- a/xen/arch/arm/include/asm/lpae.h
+++ b/xen/arch/arm/include/asm/lpae.h
@@ -4,6 +4,7 @@
#ifndef __ASSEMBLY__
#include
+#include
/*
* WARNING! Unlike the x86 pagetable code, where l1 is the lowest level and
@@ -168,6 +169,13 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned
int level)
third_table_offset(addr)\
}
+/*
+ * Standard entry type that we'll use to build Xen's own pagetables.
+ * We put the same permissions at every level, because they're ignored
+ * by the walker in non-leaf entries.
+ */
+lpae_t mfn_to_xen_entry(mfn_t mfn, unsigned int attr);
+
#endif /* __ASSEMBLY__ */
/*
diff --git a/xen/arch/arm/include/asm/pmap.h b/xen/arch/arm/include/asm/pmap.h
new file mode 100644
index ..70eafe2891d7
--- /dev/null
+++ b/xen/arch/arm/include/asm/pmap.h
@@ -0,0 +1,33 @@
+#ifndef __ASM_PMAP_H__
+#define __ASM_PMAP_H__
+
+#include
+
+/* XXX: Find an header to declare it */
+extern lpae_t xen_fixmap[XEN_PT_LPAE_ENTRIES];
+
+static inline void arch_pmap_map(unsigned int slot, mfn_t mfn)
+{
+lpae_t *entry = &xen_fixmap[slot];
+lpae_t pte;
+
+ASSERT(!lpae_is_valid(*entry));
+
+pte = mfn_to_xen_entry(mfn, PAGE_HYPERVISOR_RW);
+pte.pt.table = 1;
+write_pte(entry, pte);
+}
+
+static inline void arch_pmap_unmap(unsigned int slot)
+{
+lpae_
[PATCH v3 18/19] xen/arm: mm: Rework setup_xenheap_mappings()
From: Julien Grall
The current implementation of setup_xenheap_mappings() is using 1GB
mappings. This can lead to unexpected result because the mapping
may alias a non-cachable region (such as device or reserved regions).
For more details see B2.8 in ARM DDI 0487H.a.
map_pages_to_xen() was recently reworked to allow superpage mappings,
support contiguous mapping and deal with the use of pagge-tables before
they are mapped.
Most of the code in setup_xenheap_mappings() is now replaced with a
single call to map_pages_to_xen().
Signed-off-by: Julien Grall
Signed-off-by: Julien Grall
---
Changes in v3:
- Don't use 1GB mapping
- Re-order code in setup_mm() in a separate patch
Changes in v2:
- New patch
---
xen/arch/arm/mm.c | 87 ++-
1 file changed, 18 insertions(+), 69 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 11b6b60a2bc1..4af59375d998 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -138,17 +138,6 @@ static DEFINE_PAGE_TABLE(cpu0_pgtable);
static DEFINE_PAGE_TABLES(cpu0_dommap, DOMHEAP_SECOND_PAGES);
#endif
-#ifdef CONFIG_ARM_64
-/* The first page of the first level mapping of the xenheap. The
- * subsequent xenheap first level pages are dynamically allocated, but
- * we need this one to bootstrap ourselves. */
-static DEFINE_PAGE_TABLE(xenheap_first_first);
-/* The zeroeth level slot which uses xenheap_first_first. Used because
- * setup_xenheap_mappings otherwise relies on mfn_to_virt which isn't
- * valid for a non-xenheap mapping. */
-static __initdata int xenheap_first_first_slot = -1;
-#endif
-
/* Common pagetable leaves */
/* Second level page tables.
*
@@ -815,77 +804,37 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
void __init setup_xenheap_mappings(unsigned long base_mfn,
unsigned long nr_mfns)
{
-lpae_t *first, pte;
-unsigned long mfn, end_mfn;
-vaddr_t vaddr;
-
-/* Align to previous 1GB boundary */
-mfn = base_mfn & ~((FIRST_SIZE>>PAGE_SHIFT)-1);
+int rc;
/* First call sets the xenheap physical and virtual offset. */
if ( mfn_eq(xenheap_mfn_start, INVALID_MFN) )
{
+unsigned long mfn_gb = base_mfn & ~((FIRST_SIZE >> PAGE_SHIFT) - 1);
+
xenheap_mfn_start = _mfn(base_mfn);
xenheap_base_pdx = mfn_to_pdx(_mfn(base_mfn));
+/*
+ * The base address may not be aligned to the first level
+ * size (e.g. 1GB when using 4KB pages). This would prevent
+ * superpage mappings for all the regions because the virtual
+ * address and machine address should both be suitably aligned.
+ *
+ * Prevent that by offsetting the start of the xenheap virtual
+ * address.
+ */
xenheap_virt_start = DIRECTMAP_VIRT_START +
-(base_mfn - mfn) * PAGE_SIZE;
+(base_mfn - mfn_gb) * PAGE_SIZE;
}
if ( base_mfn < mfn_x(xenheap_mfn_start) )
panic("cannot add xenheap mapping at %lx below heap start %lx\n",
base_mfn, mfn_x(xenheap_mfn_start));
-end_mfn = base_mfn + nr_mfns;
-
-/*
- * Virtual address aligned to previous 1GB to match physical
- * address alignment done above.
- */
-vaddr = (vaddr_t)__mfn_to_virt(base_mfn) & FIRST_MASK;
-
-while ( mfn < end_mfn )
-{
-int slot = zeroeth_table_offset(vaddr);
-lpae_t *p = &xen_pgtable[slot];
-
-if ( p->pt.valid )
-{
-/* mfn_to_virt is not valid on the 1st 1st mfn, since it
- * is not within the xenheap. */
-first = slot == xenheap_first_first_slot ?
-xenheap_first_first : mfn_to_virt(lpae_get_mfn(*p));
-}
-else if ( xenheap_first_first_slot == -1)
-{
-/* Use xenheap_first_first to bootstrap the mappings */
-first = xenheap_first_first;
-
-pte = pte_of_xenaddr((vaddr_t)xenheap_first_first);
-pte.pt.table = 1;
-write_pte(p, pte);
-
-xenheap_first_first_slot = slot;
-}
-else
-{
-mfn_t first_mfn = alloc_boot_pages(1, 1);
-
-clear_page(mfn_to_virt(first_mfn));
-pte = mfn_to_xen_entry(first_mfn, MT_NORMAL);
-pte.pt.table = 1;
-write_pte(p, pte);
-first = mfn_to_virt(first_mfn);
-}
-
-pte = mfn_to_xen_entry(_mfn(mfn), MT_NORMAL);
-/* TODO: Set pte.pt.contig when appropriate. */
-write_pte(&first[first_table_offset(vaddr)], pte);
-
-mfn += FIRST_SIZE>>PAGE_SHIFT;
-vaddr += FIRST_SIZE;
-}
-
-flush_xen_tlb_local();
+rc = map_pages_to_xen((vaddr_t)__mfn_to_virt(base_mfn),
+ _mfn(base_mfn), nr_mfns,
+ PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
+if ( rc )
+panic("Unable to setup the xenheap mappings.\n");
}
[PATCH v3 17/19] xen/arm64: mm: Add memory to the boot allocator first
From: Julien Grall
Currently, memory is added to the boot allocator after the xenheap
mappings are done. This will break if the first mapping is more than
512GB of RAM.
In addition to that, a follow-up patch will rework setup_xenheap_mappings()
to use smaller mappings (e.g. 2MB, 4KB). So it will be necessary to have
memory in the boot allocator earlier.
Only free memory (e.g. not reserved or modules) can be added to the boot
allocator. It might be possible that some regions (including the first
one) will have no free memory.
So we need to add all the free memory to the boot allocator first
and then add do the mappings.
Signed-off-by: Julien Grall
---
Changes in v3:
- Patch added
---
xen/arch/arm/setup.c | 63 +---
1 file changed, 42 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index d5d0792ed48a..777cf96639f5 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -767,30 +767,18 @@ static void __init setup_mm(void)
init_staticmem_pages();
}
#else /* CONFIG_ARM_64 */
-static void __init setup_mm(void)
+static void __init populate_boot_allocator(void)
{
-paddr_t ram_start = ~0;
-paddr_t ram_end = 0;
-paddr_t ram_size = 0;
-int bank;
-
-init_pdx();
+unsigned int i;
+const struct meminfo *banks = &bootinfo.mem;
-total_pages = 0;
-for ( bank = 0 ; bank < bootinfo.mem.nr_banks; bank++ )
+for ( i = 0; i < banks->nr_banks; i++ )
{
-paddr_t bank_start = bootinfo.mem.bank[bank].start;
-paddr_t bank_size = bootinfo.mem.bank[bank].size;
-paddr_t bank_end = bank_start + bank_size;
+const struct membank *bank = &banks->bank[i];
+paddr_t bank_end = bank->start + bank->size;
paddr_t s, e;
-ram_size = ram_size + bank_size;
-ram_start = min(ram_start,bank_start);
-ram_end = max(ram_end,bank_end);
-
-setup_xenheap_mappings(bank_start>>PAGE_SHIFT, bank_size>>PAGE_SHIFT);
-
-s = bank_start;
+s = bank->start;
while ( s < bank_end )
{
paddr_t n = bank_end;
@@ -798,9 +786,7 @@ static void __init setup_mm(void)
e = next_module(s, &n);
if ( e == ~(paddr_t)0 )
-{
e = n = bank_end;
-}
if ( e > bank_end )
e = bank_end;
@@ -809,6 +795,41 @@ static void __init setup_mm(void)
s = n;
}
}
+}
+
+static void __init setup_mm(void)
+{
+const struct meminfo *banks = &bootinfo.mem;
+paddr_t ram_start = ~0;
+paddr_t ram_end = 0;
+paddr_t ram_size = 0;
+unsigned int i;
+
+init_pdx();
+
+/*
+ * We need some memory to allocate the page-tables used for the xenheap
+ * mappings. But some regions may contain memory already allocated
+ * for other uses (e.g. modules, reserved-memory...).
+ *
+ * For simplify add all the free regions in the boot allocator.
+ */
+populate_boot_allocator();
+
+total_pages = 0;
+
+for ( i = 0; i < banks->nr_banks; i++ )
+{
+const struct membank *bank = &banks->bank[i];
+paddr_t bank_end = bank->start + bank->size;
+
+ram_size = ram_size + bank->size;
+ram_start = min(ram_start, bank->start);
+ram_end = max(ram_end, bank_end);
+
+setup_xenheap_mappings(PFN_DOWN(bank->start),
+ PFN_DOWN(bank->size));
+}
total_pages += ram_size >> PAGE_SHIFT;
--
2.32.0
[PATCH v3 19/19] xen/arm: mm: Re-implement setup_frame_table_mappings() with map_pages_to_xen()
From: Julien Grall
Now that map_pages_to_xen() has been extended to support 2MB mappings,
we can replace the create_mappings() call by map_pages_to_xen() call.
This has the advantage to remove the differences between 32-bit and
64-bit code.
Lastly remove create_mappings() as there is no more callers.
Signed-off-by: Julien Grall
Signed-off-by: Julien Grall
---
Changes in v3:
- Fix typo in the commit message
- Remove the TODO regarding contiguous bit
Changes in v2:
- New patch
---
xen/arch/arm/mm.c | 63 ---
1 file changed, 5 insertions(+), 58 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 4af59375d998..d73f49d5b6fc 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -354,40 +354,6 @@ void clear_fixmap(unsigned map)
BUG_ON(res != 0);
}
-/* Create Xen's mappings of memory.
- * Mapping_size must be either 2MB or 32MB.
- * Base and virt must be mapping_size aligned.
- * Size must be a multiple of mapping_size.
- * second must be a contiguous set of second level page tables
- * covering the region starting at virt_offset. */
-static void __init create_mappings(lpae_t *second,
- unsigned long virt_offset,
- unsigned long base_mfn,
- unsigned long nr_mfns,
- unsigned int mapping_size)
-{
-unsigned long i, count;
-const unsigned long granularity = mapping_size >> PAGE_SHIFT;
-lpae_t pte, *p;
-
-ASSERT((mapping_size == MB(2)) || (mapping_size == MB(32)));
-ASSERT(!((virt_offset >> PAGE_SHIFT) % granularity));
-ASSERT(!(base_mfn % granularity));
-ASSERT(!(nr_mfns % granularity));
-
-count = nr_mfns / XEN_PT_LPAE_ENTRIES;
-p = second + second_linear_offset(virt_offset);
-pte = mfn_to_xen_entry(_mfn(base_mfn), MT_NORMAL);
-if ( granularity == 16 * XEN_PT_LPAE_ENTRIES )
-pte.pt.contig = 1; /* These maps are in 16-entry contiguous chunks. */
-for ( i = 0; i < count; i++ )
-{
-write_pte(p + i, pte);
-pte.pt.base += 1 << XEN_PT_LPAE_SHIFT;
-}
-flush_xen_tlb_local();
-}
-
#ifdef CONFIG_DOMAIN_PAGE
void *map_domain_page_global(mfn_t mfn)
{
@@ -846,36 +812,17 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t
pe)
unsigned long frametable_size = nr_pdxs * sizeof(struct page_info);
mfn_t base_mfn;
const unsigned long mapping_size = frametable_size < MB(32) ? MB(2) :
MB(32);
-#ifdef CONFIG_ARM_64
-lpae_t *second, pte;
-unsigned long nr_second;
-mfn_t second_base;
-int i;
-#endif
+int rc;
frametable_base_pdx = mfn_to_pdx(maddr_to_mfn(ps));
/* Round up to 2M or 32M boundary, as appropriate. */
frametable_size = ROUNDUP(frametable_size, mapping_size);
base_mfn = alloc_boot_pages(frametable_size >> PAGE_SHIFT, 32<<(20-12));
-#ifdef CONFIG_ARM_64
-/* Compute the number of second level pages. */
-nr_second = ROUNDUP(frametable_size, FIRST_SIZE) >> FIRST_SHIFT;
-second_base = alloc_boot_pages(nr_second, 1);
-second = mfn_to_virt(second_base);
-for ( i = 0; i < nr_second; i++ )
-{
-clear_page(mfn_to_virt(mfn_add(second_base, i)));
-pte = mfn_to_xen_entry(mfn_add(second_base, i), MT_NORMAL);
-pte.pt.table = 1;
-write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i],
pte);
-}
-create_mappings(second, 0, mfn_x(base_mfn), frametable_size >> PAGE_SHIFT,
-mapping_size);
-#else
-create_mappings(xen_second, FRAMETABLE_VIRT_START, mfn_x(base_mfn),
-frametable_size >> PAGE_SHIFT, mapping_size);
-#endif
+rc = map_pages_to_xen(FRAMETABLE_VIRT_START, base_mfn,
+ frametable_size >> PAGE_SHIFT, PAGE_HYPERVISOR_RW);
+if ( rc )
+panic("Unable to setup the frametable mappings.\n");
memset(&frame_table[0], 0, nr_pdxs * sizeof(struct page_info));
memset(&frame_table[nr_pdxs], -1,
--
2.32.0
[PATCH v3 16/19] xen/arm: mm: Use the PMAP helpers in xen_{,un}map_table()
From: Julien Grall
During early boot, it is not possible to use xen_{,un}map_table()
if the page tables are not residing the Xen binary.
This is a blocker to switch some of the helpers to use xen_pt_update()
as we may need to allocate extra page tables and access them before
the domheap has been initialized (see setup_xenheap_mappings()).
xen_{,un}map_table() are now updated to use the PMAP helpers for early
boot map/unmap. Note that the special case for page-tables residing
in Xen binary has been dropped because it is "complex" and was
only added as a workaround in 8d4f1b8878e0 ("xen/arm: mm: Allow
generic xen page-tables helpers to be called early").
Signed-off-by: Julien Grall
---
Changes in v2:
- New patch
---
xen/arch/arm/mm.c | 33 +
1 file changed, 9 insertions(+), 24 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 659bdf25e0ff..11b6b60a2bc1 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -964,27 +965,11 @@ void *ioremap(paddr_t pa, size_t len)
static lpae_t *xen_map_table(mfn_t mfn)
{
/*
- * We may require to map the page table before map_domain_page() is
- * useable. The requirements here is it must be useable as soon as
- * page-tables are allocated dynamically via alloc_boot_pages().
- *
- * We need to do the check on physical address rather than virtual
- * address to avoid truncation on Arm32. Therefore is_kernel() cannot
- * be used.
+ * During early boot, map_domain_page() may be unusable. Use the
+ * PMAP to map temporarily a page-table.
*/
if ( system_state == SYS_STATE_early_boot )
-{
-if ( is_xen_fixed_mfn(mfn) )
-{
-/*
- * It is fine to demote the type because the size of Xen
- * will always fit in vaddr_t.
- */
-vaddr_t offset = mfn_to_maddr(mfn) - virt_to_maddr(&_start);
-
-return (lpae_t *)(XEN_VIRT_START + offset);
-}
-}
+return pmap_map(mfn);
return map_domain_page(mfn);
}
@@ -993,12 +978,12 @@ static void xen_unmap_table(const lpae_t *table)
{
/*
* During early boot, xen_map_table() will not use map_domain_page()
- * for page-tables residing in Xen binary. So skip the unmap part.
+ * but the PMAP.
*/
-if ( system_state == SYS_STATE_early_boot && is_kernel(table) )
-return;
-
-unmap_domain_page(table);
+if ( system_state == SYS_STATE_early_boot )
+pmap_unmap(table);
+else
+unmap_domain_page(table);
}
static int create_xen_table(lpae_t *entry)
--
2.32.0
[PATCH v3 11/19] xen/arm: mm: Allocate xen page tables in domheap rather than xenheap
From: Julien Grall
xen_{un,}map_table() already uses the helper to map/unmap pages
on-demand (note this is currently a NOP on arm64). So switching to
domheap don't have any disavantage.
But this as the benefit:
- to keep the page tables unmapped if an arch decided to do so
- reduce xenheap use on arm32 which can be pretty small
Signed-off-by: Julien Grall
Acked-by: Stefano Stabellini
---
Changes in v3:
- Add Stefano's acked-by
Changes in v2:
- New patch
---
xen/arch/arm/mm.c | 36 +---
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 1e5c2c45dcf9..58364bb6c820 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -972,21 +972,6 @@ void *ioremap(paddr_t pa, size_t len)
return ioremap_attr(pa, len, PAGE_HYPERVISOR_NOCACHE);
}
-static int create_xen_table(lpae_t *entry)
-{
-void *p;
-lpae_t pte;
-
-p = alloc_xenheap_page();
-if ( p == NULL )
-return -ENOMEM;
-clear_page(p);
-pte = mfn_to_xen_entry(virt_to_mfn(p), MT_NORMAL);
-pte.pt.table = 1;
-write_pte(entry, pte);
-return 0;
-}
-
static lpae_t *xen_map_table(mfn_t mfn)
{
/*
@@ -1027,6 +1012,27 @@ static void xen_unmap_table(const lpae_t *table)
unmap_domain_page(table);
}
+static int create_xen_table(lpae_t *entry)
+{
+struct page_info *pg;
+void *p;
+lpae_t pte;
+
+pg = alloc_domheap_page(NULL, 0);
+if ( pg == NULL )
+return -ENOMEM;
+
+p = xen_map_table(page_to_mfn(pg));
+clear_page(p);
+xen_unmap_table(p);
+
+pte = mfn_to_xen_entry(page_to_mfn(pg), MT_NORMAL);
+pte.pt.table = 1;
+write_pte(entry, pte);
+
+return 0;
+}
+
#define XEN_TABLE_MAP_FAILED 0
#define XEN_TABLE_SUPER_PAGE 1
#define XEN_TABLE_NORMAL_PAGE 2
--
2.32.0
[PATCH v3 10/19] xen/arm32: mm: Re-implement setup_xenheap_mappings() using map_pages_to_xen()
From: Julien Grall
Now that map_pages_to_xen() has been extended to support 2MB mappings,
we can replace the create_mappings() call by map_pages_to_xen() call.
Signed-off-by: Julien Grall
---
Changes in v3:
- Fix build when CONFIG_DEBUG=y
Changes in v2:
- New patch
TODOs:
- add support for contiguous mapping
---
xen/arch/arm/mm.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index f18f65745595..1e5c2c45dcf9 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -809,7 +809,12 @@ void mmu_init_secondary_cpu(void)
void __init setup_xenheap_mappings(unsigned long base_mfn,
unsigned long nr_mfns)
{
-create_mappings(xen_second, XENHEAP_VIRT_START, base_mfn, nr_mfns, MB(32));
+int rc;
+
+rc = map_pages_to_xen(XENHEAP_VIRT_START, _mfn(base_mfn), nr_mfns,
+ PAGE_HYPERVISOR_RW | _PAGE_BLOCK);
+if ( rc )
+panic("Unable to setup the xenheap mappings.\n");
/* Record where the xenheap is, for translation routines. */
xenheap_virt_end = XENHEAP_VIRT_START + nr_mfns * PAGE_SIZE;
--
2.32.0
[PATCH v3 13/19] xen/arm: Move fixmap definitions in a separate header
From: Julien Grall
To use properly the fixmap definitions, their user would need
also new to include . This is not very great when
the user itself is not meant to directly use ACPI definitions.
Including in is not option because
the latter header is included by everyone. So move out the fixmap
entries definition in a new header.
Take the opportunity to also move {set, clear}_fixmap() prototypes
in the new header.
Note that most of the definitions in now need to be
surrounded with #ifndef __ASSEMBLY__ because will
be used in assembly (see EARLY_UART_VIRTUAL_ADDRESS).
The split will become more helpful in a follow-up patch where new
fixmap entries will be defined.
Signed-off-by: Julien Grall
---
Changes in v3:
- Patch added
---
xen/arch/arm/acpi/lib.c | 2 ++
xen/arch/arm/include/asm/config.h | 6 --
xen/arch/arm/include/asm/early_printk.h | 1 +
xen/arch/arm/include/asm/fixmap.h | 24
xen/arch/arm/include/asm/mm.h | 4
xen/arch/arm/kernel.c | 1 +
xen/arch/arm/mm.c | 1 +
xen/include/xen/acpi.h | 18 +++---
8 files changed, 40 insertions(+), 17 deletions(-)
create mode 100644 xen/arch/arm/include/asm/fixmap.h
diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
index a59cc4074cfb..41d521f720ac 100644
--- a/xen/arch/arm/acpi/lib.c
+++ b/xen/arch/arm/acpi/lib.c
@@ -25,6 +25,8 @@
#include
#include
+#include
+
static bool fixmap_inuse;
char *__acpi_map_table(paddr_t phys, unsigned long size)
diff --git a/xen/arch/arm/include/asm/config.h
b/xen/arch/arm/include/asm/config.h
index 85d4a510ce8a..51908bf9422c 100644
--- a/xen/arch/arm/include/asm/config.h
+++ b/xen/arch/arm/include/asm/config.h
@@ -175,12 +175,6 @@
#endif
-/* Fixmap slots */
-#define FIXMAP_CONSOLE 0 /* The primary UART */
-#define FIXMAP_MISC 1 /* Ephemeral mappings of hardware */
-#define FIXMAP_ACPI_BEGIN 2 /* Start mappings of ACPI tables */
-#define FIXMAP_ACPI_END(FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1) /*
End mappings of ACPI tables */
-
#define NR_hypercalls 64
#define STACK_ORDER 3
diff --git a/xen/arch/arm/include/asm/early_printk.h
b/xen/arch/arm/include/asm/early_printk.h
index 8dc911cf48a3..c5149b2976da 100644
--- a/xen/arch/arm/include/asm/early_printk.h
+++ b/xen/arch/arm/include/asm/early_printk.h
@@ -11,6 +11,7 @@
#define __ARM_EARLY_PRINTK_H__
#include
+#include
#ifdef CONFIG_EARLY_PRINTK
diff --git a/xen/arch/arm/include/asm/fixmap.h
b/xen/arch/arm/include/asm/fixmap.h
new file mode 100644
index ..1cee51e52ab9
--- /dev/null
+++ b/xen/arch/arm/include/asm/fixmap.h
@@ -0,0 +1,24 @@
+/*
+ * fixmap.h: compile-time virtual memory allocation
+ */
+#ifndef __ASM_FIXMAP_H
+#define __ASM_FIXMAP_H
+
+#include
+
+/* Fixmap slots */
+#define FIXMAP_CONSOLE 0 /* The primary UART */
+#define FIXMAP_MISC 1 /* Ephemeral mappings of hardware */
+#define FIXMAP_ACPI_BEGIN 2 /* Start mappings of ACPI tables */
+#define FIXMAP_ACPI_END(FIXMAP_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1) /*
End mappings of ACPI tables */
+
+#ifndef __ASSEMBLY__
+
+/* Map a page in a fixmap entry */
+extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
+/* Remove a mapping from a fixmap entry */
+extern void clear_fixmap(unsigned map);
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_FIXMAP_H */
diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 424aaf28230b..045a8ba4bb63 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -191,10 +191,6 @@ extern void mmu_init_secondary_cpu(void);
extern void setup_xenheap_mappings(unsigned long base_mfn, unsigned long
nr_mfns);
/* Map a frame table to cover physical addresses ps through pe */
extern void setup_frametable_mappings(paddr_t ps, paddr_t pe);
-/* Map a 4k page in a fixmap entry */
-extern void set_fixmap(unsigned map, mfn_t mfn, unsigned attributes);
-/* Remove a mapping from a fixmap entry */
-extern void clear_fixmap(unsigned map);
/* map a physical range in virtual memory */
void __iomem *ioremap_attr(paddr_t start, size_t len, unsigned attributes);
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 8f43caa1866d..25ded1c056d9 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -15,6 +15,7 @@
#include
#include
+#include
#include
#include
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index f70b8cc7ce87..d6a4b9407c43 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -41,6 +41,7 @@
#include
#include
+#include
#include
/* Override macros from asm/page.h to make them work with mfn_t */
diff --git a/xen/include/xen/acpi.h b/xen/include/xen/acpi.h
index 08834f140266..500aaa538551 100644
--- a/xen/include/xen/acpi.h
+++ b/xen/include/xen/acpi.h
@@ -28,6 +28,15 @@
#define _LINUX
#endif
+/*
+ * Fixmap pages to reserve
[PATCH v3 15/19] xen/arm: mm: Clean-up the includes and order them
From: Julien Grall The numbers of includes in mm.c has been growing quite a lot. However some of them (e.g. xen/device_tree.h, xen/softirq.h) doesn't look to be directly used by the file or other will be included by larger headers (e.g asm/flushtlb.h will be included by xen/mm.h). So trim down the number of includes. Take the opportunity to order them with the xen headers first, then asm headers and last public headers. Signed-off-by: Julien Grall --- Changes in v3: - Patch added --- xen/arch/arm/mm.c | 27 ++- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index b7942464d4de..659bdf25e0ff 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -17,33 +17,26 @@ * GNU General Public License for more details. */ -#include -#include -#include -#include -#include -#include +#include #include #include -#include -#include #include -#include -#include -#include -#include -#include -#include +#include +#include +#include +#include #include +#include +#include #include + #include -#include -#include -#include #include #include +#include + /* Override macros from asm/page.h to make them work with mfn_t */ #undef virt_to_mfn #define virt_to_mfn(va) _mfn(__virt_to_mfn(va)) -- 2.32.0
[PATCH v3 12/19] xen/arm: mm: Allow page-table allocation from the boot allocator
From: Julien Grall
At the moment, page-table can only be allocated from domheap. This means
it is not possible to create mapping in the page-tables via
map_pages_to_xen() if page-table needs to be allocated.
In order to avoid open-coding page-tables update in early boot, we need
to be able to allocate page-tables much earlier. Thankfully, we have the
boot allocator for those cases.
create_xen_table() is updated to cater early boot allocation by using
alloc_boot_pages().
Note, this is not sufficient to bootstrap the page-tables (i.e mapping
before any memory is actually mapped). This will be addressed
separately.
Signed-off-by: Julien Grall
Signed-off-by: Julien Grall
---
Changes in v2:
- New patch
---
xen/arch/arm/mm.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 58364bb6c820..f70b8cc7ce87 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1014,19 +1014,27 @@ static void xen_unmap_table(const lpae_t *table)
static int create_xen_table(lpae_t *entry)
{
-struct page_info *pg;
+mfn_t mfn;
void *p;
lpae_t pte;
-pg = alloc_domheap_page(NULL, 0);
-if ( pg == NULL )
-return -ENOMEM;
+if ( system_state != SYS_STATE_early_boot )
+{
+struct page_info *pg = alloc_domheap_page(NULL, 0);
+
+if ( pg == NULL )
+return -ENOMEM;
+
+mfn = page_to_mfn(pg);
+}
+else
+mfn = alloc_boot_pages(1, 1);
-p = xen_map_table(page_to_mfn(pg));
+p = xen_map_table(mfn);
clear_page(p);
xen_unmap_table(p);
-pte = mfn_to_xen_entry(page_to_mfn(pg), MT_NORMAL);
+pte = mfn_to_xen_entry(mfn, MT_NORMAL);
pte.pt.table = 1;
write_pte(entry, pte);
--
2.32.0
Re: [PATCH 1/3] tests/resource: Initialise gnttab before xenforeignmemory_map_resource()
On Mon, Feb 21, 2022 at 10:02:52AM +, Andrew Cooper wrote:
> It the 'addr' input to mmap(), and currently consuming stack rubble.
>
> Coverity-ID: 1500115
> Fixes: c7a7f14b9299 ("tests/resource: Extend to check that the grant frames
> are mapped correctly")
> Signed-off-by: Andrew Cooper
Reviewed-by: Roger Pau Monné
Thanks, Roger.
Re: [PATCH 3/3] CI: Coverity tweaks
On Mon, Feb 21, 2022 at 10:02:54AM +, Andrew Cooper wrote:
> * Use workflow_dispatch to allow manual creation of the job.
> * Use parallel builds. The workers have two vCPUs.
> * Shrink the dependency list further. build-essential covers make and gcc,
>while bridge-utils and iproute2 are runtime dependencies not build
>dependencies. Alter bzip2 to libbz2-dev.
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Roger Pau Monné
> ---
> .github/workflows/coverity.yml | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
> index 9d04b56fd31d..6e7b81e74f72 100644
> --- a/.github/workflows/coverity.yml
> +++ b/.github/workflows/coverity.yml
> @@ -2,6 +2,7 @@ name: Coverity Scan
>
> # We only want to test official release code, not every pull request.
> on:
> + workflow_dispatch:
>schedule:
> - cron: '18 9 * * WED,SUN' # Bi-weekly at 9:18 UTC
>
> @@ -11,11 +12,11 @@ jobs:
> steps:
> - name: Install build dependencies
>run: |
> -sudo apt-get install -y wget git gawk bridge-utils \
> - iproute2 bzip2 build-essential \
> - make gcc zlib1g-dev libncurses5-dev iasl \
> - libbz2-dev e2fslibs-dev git-core uuid-dev ocaml \
> - ocaml-findlib xz-utils libyajl-dev \
> +sudo apt-get install -y wget git gawk \
> + libbz2-dev build-essential \
> + zlib1g-dev libncurses5-dev iasl \
> + libbz2-dev e2fslibs-dev uuid-dev ocaml \
> + ocaml-findlib libyajl-dev \
>autoconf libtool liblzma-dev \
>python3-dev golang python-dev libsystemd-dev
>
> @@ -31,7 +32,7 @@ jobs:
>
> - name: Pre build stuff
>run: |
> -make mini-os-dir
> +make -j`nproc` mini-os-dir
>
> - uses: vapier/coverity-scan-action@v1
>with:
> @@ -39,3 +40,4 @@ jobs:
> project: XenProject
> email: ${{ secrets.COVERITY_SCAN_EMAIL }}
> token: ${{ secrets.COVERITY_SCAN_TOKEN }}
> +command: make -j`nproc` build
There's already a 'command:' parameter set just before 'project:'. Are
we OK with using plain build?
If so we would have to disable docs build and stubdom? We don't want
to analyze all the newlib &c that's build as part of stubdoms?
Anyway, the switch from `make xen tools && make -C extras/mini-os/`
to `make build` needs to be explained in the commit message IMO.
Thanks, Roger.
[xen-unstable test] 168182: tolerable FAIL
flight 168182 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/168182/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-arm64-arm64-xl-seattle 8 xen-boot fail in 168175 pass in 168182 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass in 168175 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 168175 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168175 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168175 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 168175 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeatfail like 168175 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 168175 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 168175 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 168175 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 168175 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 168175 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 168175 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 168175 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168175 test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test
[PATCH v3 02/19] xen/arm: lpae: Use the generic helpers to defined the Xen PT helpers
From: Julien Grall Currently, Xen PT helpers are only working with 4KB page granularity and open-code the generic helpers. To allow more flexibility, we can re-use the generic helpers and pass Xen's page granularity (PAGE_SHIFT). As Xen PT helpers are used in both C and assembly, we need to move the generic helpers definition outside of the !__ASSEMBLY__ section. Take the opportunity to prefix LPAE_ENTRIES, LPAE_ENTRIES and LPAE_ENTRIES_MASK with XEN_PT_. Note the aliases for each level are still kept for the time being so we can avoid a massive patch to change all the callers. Signed-off-by: Julien Grall --- Changes in v3: - Prefix the new define with XEN_PT_ Changes in v2: - New patch --- xen/arch/arm/arm32/head.S | 14 +++ xen/arch/arm/arm64/head.S | 14 +++ xen/arch/arm/include/asm/lpae.h | 73 ++--- xen/arch/arm/mm.c | 33 --- xen/arch/arm/p2m.c | 13 +++--- 5 files changed, 80 insertions(+), 67 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index b5912d381b98..b1d209ea2842 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -375,7 +375,7 @@ ENDPROC(cpu_init) */ .macro create_table_entry, ptbl, tbl, virt, shift, mmu=0 lsr r1, \virt, #\shift -mov_w r2, LPAE_ENTRY_MASK +mov_w r2, XEN_PT_LPAE_ENTRY_MASK and r1, r1, r2 /* r1 := slot in \tlb */ lsl r1, r1, #3 /* r1 := slot offset in \tlb */ @@ -410,7 +410,7 @@ ENDPROC(cpu_init) * and be distinct. */ .macro create_mapping_entry, ptbl, virt, phys, type=PT_MEM_L3, mmu=0 -mov_w r2, LPAE_ENTRY_MASK +mov_w r2, XEN_PT_LPAE_ENTRY_MASK lsr r1, \virt, #THIRD_SHIFT and r1, r1, r2 /* r1 := slot in \tlb */ lsl r1, r1, #3 /* r1 := slot offset in \tlb */ @@ -465,7 +465,7 @@ create_page_tables: 1: strd r2, r3, [r4, r1] /* Map vaddr(start) */ add r2, r2, #PAGE_SIZE /* Next page */ add r1, r1, #8 /* Next slot */ -cmp r1, #(LPAE_ENTRIES<<3) /* 512*8-byte entries per page */ +cmp r1, #(XEN_PT_LPAE_ENTRIES<<3) /* 512*8-byte entries per page */ blo 1b /* @@ -487,7 +487,7 @@ create_page_tables: * the second level. */ lsr r1, r9, #FIRST_SHIFT -mov_w r0, LPAE_ENTRY_MASK +mov_w r0, XEN_PT_LPAE_ENTRY_MASK and r1, r1, r0 /* r1 := first slot */ cmp r1, #XEN_FIRST_SLOT beq 1f @@ -502,7 +502,7 @@ create_page_tables: * it. */ lsr r1, r9, #SECOND_SHIFT -mov_w r0, LPAE_ENTRY_MASK +mov_w r0, XEN_PT_LPAE_ENTRY_MASK and r1, r1, r0 /* r1 := second slot */ cmp r1, #XEN_SECOND_SLOT beq virtphys_clash @@ -573,7 +573,7 @@ remove_identity_mapping: * table if the slot is not XEN_FIRST_SLOT. */ lsr r1, r9, #FIRST_SHIFT -mov_w r0, LPAE_ENTRY_MASK +mov_w r0, XEN_PT_LPAE_ENTRY_MASK and r1, r1, r0 /* r1 := first slot */ cmp r1, #XEN_FIRST_SLOT beq 1f @@ -589,7 +589,7 @@ remove_identity_mapping: * table if the slot is not XEN_SECOND_SLOT. */ lsr r1, r9, #SECOND_SHIFT -mov_w r0, LPAE_ENTRY_MASK +mov_w r0, XEN_PT_LPAE_ENTRY_MASK and r1, r1, r0 /* r1 := second slot */ cmp r1, #XEN_SECOND_SLOT beq identity_mapping_removed diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 51b00ab0bea6..314b800b3f8e 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -509,7 +509,7 @@ ENDPROC(cpu_init) */ .macro create_table_entry, ptbl, tbl, virt, shift, tmp1, tmp2, tmp3 lsr \tmp1, \virt, #\shift -and \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ +and \tmp1, \tmp1, #XEN_PT_LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ load_paddr \tmp2, \tbl mov \tmp3, #PT_PT /* \tmp3 := right for linear PT */ @@ -541,7 +541,7 @@ ENDPROC(cpu_init) and \tmp3, \phys, #THIRD_MASK /* \tmp3 := PAGE_ALIGNED(phys) */ lsr \tmp1, \virt, #THIRD_SHIFT -and \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ +and \tmp1, \tmp1, #XEN_PT_LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ mov \tmp2, #\type /* \tmp2 := right for section PT */ orr \tmp2, \tmp2, \tmp3 /* + PAGE_ALIGNED(phys) */ @@ -586,7 +586,7 @@ create_page_tables: 1: str x2, [x4, x1] /* Map vaddr(start) */ add x2, x2, #PAGE_SIZE /* Next page */ add x1, x1, #8 /* Next slot */ -cmp x1, #(LPAE_ENTRIES<<3) /* 512 entries per pag
[PATCH v3 09/19] xen/arm32: mm: Check if the virtual address is shared before updating it
From: Julien Grall
Only the first 2GB of the virtual address space is shared between all
the page-tables on Arm32.
There is a long outstanding TODO in xen_pt_update() stating that the
function can only work with shared mapping. Nobody has ever called
the function with private mapping, however as we add more callers
there is a risk to mess things up.
Introduce a new define to mark the end of the shared mappings and use
it in xen_pt_update() to verify if the address is correct.
Note that on Arm64, all the mappings are shared. Some compiler may
complain about an always true check, so the new define is not introduced
for arm64 and the code is protected with an #ifdef.
Signed-off-by: Julien Grall
---
Changes in v2:
- New patch
---
xen/arch/arm/include/asm/config.h | 4
xen/arch/arm/mm.c | 11 +--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/include/asm/config.h
b/xen/arch/arm/include/asm/config.h
index c7b77912013e..85d4a510ce8a 100644
--- a/xen/arch/arm/include/asm/config.h
+++ b/xen/arch/arm/include/asm/config.h
@@ -137,6 +137,10 @@
#define XENHEAP_VIRT_START _AT(vaddr_t,0x4000)
#define XENHEAP_VIRT_END _AT(vaddr_t,0x7fff)
+
+/* The first 2GB is always shared between all the page-tables. */
+#define SHARED_VIRT_END_AT(vaddr_t, 0x7fff)
+
#define DOMHEAP_VIRT_START _AT(vaddr_t,0x8000)
#define DOMHEAP_VIRT_END _AT(vaddr_t,0x)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 24de8dcb9042..f18f65745595 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1365,11 +1365,18 @@ static int xen_pt_update(unsigned long virt,
* For arm32, page-tables are different on each CPUs. Yet, they share
* some common mappings. It is assumed that only common mappings
* will be modified with this function.
- *
- * XXX: Add a check.
*/
const mfn_t root = virt_to_mfn(THIS_CPU_PGTABLE);
+#ifdef SHARED_VIRT_END
+if ( virt > SHARED_VIRT_END ||
+ (SHARED_VIRT_END - virt) < nr_mfns )
+{
+mm_printk("Trying to map outside of the shared area.\n");
+return -EINVAL;
+}
+#endif
+
/*
* The hardware was configured to forbid mapping both writeable and
* executable.
--
2.32.0
[PATCH v3 08/19] xen/arm: mm: Re-implement early_fdt_map() using map_pages_to_xen()
From: Julien Grall
Now that map_pages_to_xen() has been extended to support 2MB mappings,
we can replace the create_mappings() calls by map_pages_to_xen() calls.
The mapping can also be marked read-only has Xen as no business to
modify the host Device Tree.
Signed-off-by: Julien Grall
Signed-off-by: Julien Grall
---
Changes in v2:
- Add my AWS signed-off-by
- Fix typo in the commit message
---
xen/arch/arm/mm.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index f088a4b2de96..24de8dcb9042 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -559,6 +559,7 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
paddr_t offset;
void *fdt_virt;
uint32_t size;
+int rc;
/*
* Check whether the physical FDT address is set and meets the minimum
@@ -574,8 +575,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
/* The FDT is mapped using 2MB superpage */
BUILD_BUG_ON(BOOT_FDT_VIRT_START % SZ_2M);
-create_mappings(xen_second, BOOT_FDT_VIRT_START, paddr_to_pfn(base_paddr),
-SZ_2M >> PAGE_SHIFT, SZ_2M);
+rc = map_pages_to_xen(BOOT_FDT_VIRT_START, maddr_to_mfn(base_paddr),
+ SZ_2M >> PAGE_SHIFT,
+ PAGE_HYPERVISOR_RO | _PAGE_BLOCK);
+if ( rc )
+panic("Unable to map the device-tree.\n");
+
offset = fdt_paddr % SECOND_SIZE;
fdt_virt = (void *)BOOT_FDT_VIRT_START + offset;
@@ -589,9 +594,12 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
if ( (offset + size) > SZ_2M )
{
-create_mappings(xen_second, BOOT_FDT_VIRT_START + SZ_2M,
-paddr_to_pfn(base_paddr + SZ_2M),
-SZ_2M >> PAGE_SHIFT, SZ_2M);
+rc = map_pages_to_xen(BOOT_FDT_VIRT_START + SZ_2M,
+ maddr_to_mfn(base_paddr + SZ_2M),
+ SZ_2M >> PAGE_SHIFT,
+ PAGE_HYPERVISOR_RO | _PAGE_BLOCK);
+if ( rc )
+panic("Unable to map the device-tree\n");
}
return fdt_virt;
--
2.32.0
[PATCH v3 07/19] xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings()
From: Julien Grall
Now that xen_pt_update_entry() is able to deal with different mapping
size, we can replace the open-coding of the page-tables update by a call
to modify_xen_mappings().
As the function is not meant to fail, a BUG_ON() is added to check the
return.
Signed-off-by: Julien Grall
Signed-off-by: Julien Grall
---
Changes in v2:
- Stay consistent with how function name are used in the commit
message
- Add my AWS signed-off-by
---
xen/arch/arm/mm.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7b4b9de8693e..f088a4b2de96 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -599,11 +599,11 @@ void * __init early_fdt_map(paddr_t fdt_paddr)
void __init remove_early_mappings(void)
{
-lpae_t pte = {0};
-write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START), pte);
-write_pte(xen_second + second_table_offset(BOOT_FDT_VIRT_START + SZ_2M),
- pte);
-flush_xen_tlb_range_va(BOOT_FDT_VIRT_START, BOOT_FDT_SLOT_SIZE);
+int rc;
+
+rc = modify_xen_mappings(BOOT_FDT_VIRT_START, BOOT_FDT_VIRT_END,
+ _PAGE_BLOCK);
+BUG_ON(rc);
}
/*
--
2.32.0
[PATCH v3 06/19] xen/arm: mm: Avoid flushing the TLBs when mapping are inserted
From: Julien Grall
Currently, the function xen_pt_update() will flush the TLBs even when
the mappings are inserted. This is a bit wasteful because we don't
allow mapping replacement. Even if we were, the flush would need to
happen earlier because mapping replacement should use Break-Before-Make
when updating the entry.
A single call to xen_pt_update() can perform a single action. IOW, it
is not possible to, for instance, mix inserting and removing mappings.
Therefore, we can use `flags` to determine what action is performed.
This change will be particularly help to limit the impact of switching
boot time mapping to use xen_pt_update().
Signed-off-by: Julien Grall
---
Changes in v2:
- New patch
---
xen/arch/arm/mm.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index fd16c1541ce2..7b4b9de8693e 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1104,7 +1104,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn,
unsigned int level,
/* We should be here with a valid MFN. */
ASSERT(!mfn_eq(mfn, INVALID_MFN));
-/* We don't allow replacing any valid entry. */
+/*
+ * We don't allow replacing any valid entry.
+ *
+ * Note that the function xen_pt_update() relies on this
+ * assumption and will skip the TLB flush. The function will need
+ * to be updated if the check is relaxed.
+ */
if ( lpae_is_valid(entry) )
{
if ( lpae_is_mapping(entry, level) )
@@ -1417,11 +1423,16 @@ static int xen_pt_update(unsigned long virt,
}
/*
- * Flush the TLBs even in case of failure because we may have
+ * The TLBs flush can be safely skipped when a mapping is inserted
+ * as we don't allow mapping replacement (see xen_pt_check_entry()).
+ *
+ * For all the other cases, the TLBs will be flushed unconditionally
+ * even if the mapping has failed. This is because we may have
* partially modified the PT. This will prevent any unexpected
* behavior afterwards.
*/
-flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
+if ( !(flags & _PAGE_PRESENT) || mfn_eq(mfn, INVALID_MFN) )
+flush_xen_tlb_range_va(virt, PAGE_SIZE * nr_mfns);
spin_unlock(&xen_pt_lock);
--
2.32.0
[PATCH v3 05/19] xen/arm: mm: Add support for the contiguous bit
From: Julien Grall
In follow-up patches, we will use xen_pt_update() (or its callers)
to handle large mappings (e.g. frametable, xenheap). They are also
not going to be modified once created.
The page-table entries have an hint to indicate that whether an
entry is contiguous to another 16 entries (assuming 4KB). When the
processor support the hint, one TLB entry will be created per
contiguous region.
For now this is tied to _PAGE_BLOCK. We can untie it in the future
if there are use-cases where we may want to use _PAGE_BLOCK without
setting the contiguous (couldn't think of any yet).
Note that to avoid extra complexity, mappings with the contiguous
bit set cannot be removed. Given the expected use, this restriction
ought to be fine.
Signed-off-by: Julien Grall
---
Changes in v3:
- New patch
---
xen/arch/arm/include/asm/page.h | 4 ++
xen/arch/arm/mm.c | 80 ++---
2 files changed, 77 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/include/asm/page.h b/xen/arch/arm/include/asm/page.h
index 07998df47bac..e7cd62190c7f 100644
--- a/xen/arch/arm/include/asm/page.h
+++ b/xen/arch/arm/include/asm/page.h
@@ -70,6 +70,7 @@
* [5] Page present
* [6] Only populate page tables
* [7] Superpage mappings is allowed
+ * [8] Set contiguous bit (internal flag)
*/
#define PAGE_AI_MASK(x) ((x) & 0x7U)
@@ -86,6 +87,9 @@
#define _PAGE_BLOCK_BIT 7
#define _PAGE_BLOCK (1U << _PAGE_BLOCK_BIT)
+#define _PAGE_CONTIG_BIT8
+#define _PAGE_CONTIG(1U << _PAGE_CONTIG_BIT)
+
/*
* _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
* meant to be used outside of this header.
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 3af69b396bd1..fd16c1541ce2 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1237,6 +1237,8 @@ static int xen_pt_update_entry(mfn_t root, unsigned long
virt,
/* Set permission */
pte.pt.ro = PAGE_RO_MASK(flags);
pte.pt.xn = PAGE_XN_MASK(flags);
+/* Set contiguous bit */
+pte.pt.contig = !!(flags & _PAGE_CONTIG);
}
write_pte(entry, pte);
@@ -1289,6 +1291,51 @@ static int xen_pt_mapping_level(unsigned long vfn, mfn_t
mfn, unsigned long nr,
return level;
}
+#define XEN_PT_4K_NR_CONTIG 16
+
+/*
+ * Check whether the contiguous bit can be set. Return the number of
+ * contiguous entry allowed. If not allowed, return 1.
+ */
+static unsigned int xen_pt_check_contig(unsigned long vfn, mfn_t mfn,
+unsigned int level, unsigned long left,
+unsigned int flags)
+{
+unsigned long nr_contig;
+
+/*
+ * Allow the contiguous bit to set when the caller requests block
+ * mapping.
+ */
+if ( !(flags & _PAGE_BLOCK) )
+return 1;
+
+/*
+ * We don't allow to remove mapping with the contiguous bit set.
+ * So shortcut the logic and directly return 1.
+ */
+if ( mfn_eq(mfn, INVALID_MFN) )
+return 1;
+
+/*
+ * The number of contiguous entries varies depending on the page
+ * granularity used. The logic below assumes 4KB.
+ */
+BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
+
+/*
+ * In order to enable the contiguous bit, we should have enough entries
+ * to map left and both the virtual and physical address should be
+ * aligned to the size of 16 translation tables entries.
+ */
+nr_contig = BIT(XEN_PT_LEVEL_ORDER(level), UL) * XEN_PT_4K_NR_CONTIG;
+
+if ( (left < nr_contig) || ((mfn_x(mfn) | vfn) & (nr_contig - 1)) )
+return 1;
+
+return XEN_PT_4K_NR_CONTIG;
+}
+
static DEFINE_SPINLOCK(xen_pt_lock);
static int xen_pt_update(unsigned long virt,
@@ -1322,6 +1369,12 @@ static int xen_pt_update(unsigned long virt,
return -EINVAL;
}
+if ( flags & _PAGE_CONTIG )
+{
+mm_printk("_PAGE_CONTIG is an internal only flag.\n");
+return -EINVAL;
+}
+
if ( !IS_ALIGNED(virt, PAGE_SIZE) )
{
mm_printk("The virtual address is not aligned to the page-size.\n");
@@ -1333,21 +1386,34 @@ static int xen_pt_update(unsigned long virt,
while ( left )
{
unsigned int order, level;
+unsigned int nr_contig;
+unsigned int new_flags;
level = xen_pt_mapping_level(vfn, mfn, left, flags);
order = XEN_PT_LEVEL_ORDER(level);
ASSERT(left >= BIT(order, UL));
-rc = xen_pt_update_entry(root, pfn_to_paddr(vfn), mfn, level, flags);
-if ( rc )
-break;
+/*
+ * Check if we can set the contiguous mapping and update the
+ * flags accordingly.
+ */
+nr_contig = xen_pt_check_contig(vfn, mfn, level, left, flags);
+new_flags = flags | ((nr_contig > 1) ? _PAGE_CONTIG : 0);
-vfn += 1U << order;
-if ( !mfn_eq(mfn, INVALID_MFN) )
-mfn = mfn_add(mfn, 1U << order);
+
[PATCH v3 04/19] xen/arm: mm: Allow other mapping size in xen_pt_update_entry()
From: Julien Grall
At the moment, xen_pt_update_entry() only supports mapping at level 3
(i.e 4KB mapping). While this is fine for most of the runtime helper,
the boot code will require to use superpage mapping.
We don't want to allow superpage mapping by default as some of the
callers may expect small mappings (i.e populate_pt_range()) or even
expect to unmap only a part of a superpage.
To keep the code simple, a new flag _PAGE_BLOCK is introduced to
allow the caller to enable superpage mapping.
As the code doesn't support all the combinations, xen_pt_check_entry()
is extended to take into account the cases we don't support when
using block mapping:
- Replacing a table with a mapping. This may happen if region was
first mapped with 4KB mapping and then later on replaced with a 2MB
(or 1GB mapping).
- Removing/modifying a table. This may happen if a caller try to
remove a region with _PAGE_BLOCK set when it was created without it.
Note that the current restriction means that the caller must ensure that
_PAGE_BLOCK is consistently set/cleared across all the updates on a
given virtual region. This ought to be fine with the expected use-cases.
More rework will be necessary if we wanted to remove the restrictions.
Note that nr_mfns is now marked const as it is used for flushing the
TLBs and we don't want it to be modified.
Signed-off-by: Julien Grall
Signed-off-by: Julien Grall
---
Changes in v3:
- Fix clash after prefixing the PT macros with XEN_PT_
- Fix typoes in the commit message
- Support superpage mappings even if nr is not suitably aligned
- Move the logic to find the level in a separate function
Changes in v2:
- Pass the target level rather than the order to
xen_pt_update_entry()
- Update some comments
- Open-code paddr_to_pfn()
- Add my AWS signed-off-by
---
xen/arch/arm/include/asm/page.h | 4 ++
xen/arch/arm/mm.c | 108 ++--
2 files changed, 94 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/include/asm/page.h b/xen/arch/arm/include/asm/page.h
index c6f9fb0d4e0c..07998df47bac 100644
--- a/xen/arch/arm/include/asm/page.h
+++ b/xen/arch/arm/include/asm/page.h
@@ -69,6 +69,7 @@
* [3:4] Permission flags
* [5] Page present
* [6] Only populate page tables
+ * [7] Superpage mappings is allowed
*/
#define PAGE_AI_MASK(x) ((x) & 0x7U)
@@ -82,6 +83,9 @@
#define _PAGE_PRESENT(1U << 5)
#define _PAGE_POPULATE (1U << 6)
+#define _PAGE_BLOCK_BIT 7
+#define _PAGE_BLOCK (1U << _PAGE_BLOCK_BIT)
+
/*
* _PAGE_DEVICE and _PAGE_NORMAL are convenience defines. They are not
* meant to be used outside of this header.
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 515d0906f85b..3af69b396bd1 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1063,9 +1063,10 @@ static int xen_pt_next_level(bool read_only, unsigned
int level,
}
/* Sanity check of the entry */
-static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int flags)
+static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn, unsigned int level,
+ unsigned int flags)
{
-/* Sanity check when modifying a page. */
+/* Sanity check when modifying an entry. */
if ( (flags & _PAGE_PRESENT) && mfn_eq(mfn, INVALID_MFN) )
{
/* We don't allow modifying an invalid entry. */
@@ -1075,6 +1076,13 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn,
unsigned int flags)
return false;
}
+/* We don't allow modifying a table entry */
+if ( !lpae_is_mapping(entry, level) )
+{
+mm_printk("Modifying a table entry is not allowed.\n");
+return false;
+}
+
/* We don't allow changing memory attributes. */
if ( entry.pt.ai != PAGE_AI_MASK(flags) )
{
@@ -1090,7 +1098,7 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn,
unsigned int flags)
return false;
}
}
-/* Sanity check when inserting a page */
+/* Sanity check when inserting a mapping */
else if ( flags & _PAGE_PRESENT )
{
/* We should be here with a valid MFN. */
@@ -1099,18 +1107,28 @@ static bool xen_pt_check_entry(lpae_t entry, mfn_t mfn,
unsigned int flags)
/* We don't allow replacing any valid entry. */
if ( lpae_is_valid(entry) )
{
-mm_printk("Changing MFN for a valid entry is not allowed
(%#"PRI_mfn" -> %#"PRI_mfn").\n",
- mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+if ( lpae_is_mapping(entry, level) )
+mm_printk("Changing MFN for a valid entry is not allowed
(%#"PRI_mfn" -> %#"PRI_mfn").\n",
+ mfn_x(lpae_get_mfn(entry)), mfn_x(mfn));
+else
+mm_printk("Trying to replace a table with a mapping.\n");
return false;
[PATCH v3 03/19] xen/arm: p2m: Replace level_{orders, masks} arrays with XEN_PT_LEVEL_{ORDER, MASK}
From: Julien Grall
The array level_orders and level_masks can be replaced with the
recently introduced macros LEVEL_ORDER and LEVEL_MASK.
Signed-off-by: Julien Grall
---
Changes in v3:
- Fix clashes after prefixing the PT macros with XEN_PT_
Changes in v2:
- New patch
The goal is to remove completely the static arrays so they
don't need to be global (or duplicated) when adding superpage
support for Xen PT.
This also has the added benefits to replace a couple of loads
with only a few instructions working on immediate.
---
xen/arch/arm/p2m.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 493a1e25879a..1d1059f7d2bd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -37,12 +37,6 @@ static unsigned int __read_mostly max_vmid = MAX_VMID_8_BIT;
*/
unsigned int __read_mostly p2m_ipa_bits = 64;
-/* Helpers to lookup the properties of each level */
-static const paddr_t level_masks[] =
-{ ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
-static const uint8_t level_orders[] =
-{ ZEROETH_ORDER, FIRST_ORDER, SECOND_ORDER, THIRD_ORDER };
-
static mfn_t __read_mostly empty_root_mfn;
static uint64_t generate_vttbr(uint16_t vmid, mfn_t root_mfn)
@@ -233,7 +227,7 @@ static lpae_t *p2m_get_root_pointer(struct p2m_domain *p2m,
* we can't use (P2M_ROOT_LEVEL - 1) because the root level might be
* 0. Yet we still want to check if all the unused bits are zeroed.
*/
-root_table = gfn_x(gfn) >> (level_orders[P2M_ROOT_LEVEL] +
+root_table = gfn_x(gfn) >> (XEN_PT_LEVEL_ORDER(P2M_ROOT_LEVEL) +
XEN_PT_LPAE_SHIFT);
if ( root_table >= P2M_ROOT_PAGES )
return NULL;
@@ -380,7 +374,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
if ( gfn_x(gfn) > gfn_x(p2m->max_mapped_gfn) )
{
for ( level = P2M_ROOT_LEVEL; level < 3; level++ )
-if ( (gfn_x(gfn) & (level_masks[level] >> PAGE_SHIFT)) >
+if ( (gfn_x(gfn) & (XEN_PT_LEVEL_MASK(level) >> PAGE_SHIFT)) >
gfn_x(p2m->max_mapped_gfn) )
break;
@@ -423,7 +417,8 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
* The entry may point to a superpage. Find the MFN associated
* to the GFN.
*/
-mfn = mfn_add(mfn, gfn_x(gfn) & ((1UL << level_orders[level]) - 1));
+mfn = mfn_add(mfn,
+ gfn_x(gfn) & ((1UL << XEN_PT_LEVEL_ORDER(level)) - 1));
if ( valid )
*valid = lpae_is_valid(entry);
@@ -434,7 +429,7 @@ out_unmap:
out:
if ( page_order )
-*page_order = level_orders[level];
+*page_order = XEN_PT_LEVEL_ORDER(level);
return mfn;
}
@@ -808,7 +803,7 @@ static bool p2m_split_superpage(struct p2m_domain *p2m,
lpae_t *entry,
/* Convenience aliases */
mfn_t mfn = lpae_get_mfn(*entry);
unsigned int next_level = level + 1;
-unsigned int level_order = level_orders[next_level];
+unsigned int level_order = XEN_PT_LEVEL_ORDER(next_level);
/*
* This should only be called with target != level and the entry is
--
2.32.0
[PATCH v3 00/19] xen/arm: mm: Remove open-coding mappings
From: Julien Grall
Hi all,
This series was originally sent as "xen/arm: mm: Add limited support
for superpages" [1] and finally has grown enough to remove most of
the open-coding mappings in the boot code.
This will help to:
1) Get better compliance with the Arm memory model
2) Pave the way to support other page size (64KB, 16KB)
The previous version was spent a few months ago. So I have decided
to remove all the acked-by/reviewed-by tags.
Cheers,
[1] <20201119190751.22345-1-jul...@xen.org>
[2]
Julien Grall (18):
xen/arm: lpae: Rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS
xen/arm: lpae: Use the generic helpers to defined the Xen PT helpers
xen/arm: p2m: Replace level_{orders, masks} arrays with
XEN_PT_LEVEL_{ORDER, MASK}
xen/arm: mm: Allow other mapping size in xen_pt_update_entry()
xen/arm: mm: Add support for the contiguous bit
xen/arm: mm: Avoid flushing the TLBs when mapping are inserted
xen/arm: mm: Don't open-code Xen PT update in remove_early_mappings()
xen/arm: mm: Re-implement early_fdt_map() using map_pages_to_xen()
xen/arm32: mm: Check if the virtual address is shared before updating
it
xen/arm32: mm: Re-implement setup_xenheap_mappings() using
map_pages_to_xen()
xen/arm: mm: Allocate xen page tables in domheap rather than xenheap
xen/arm: mm: Allow page-table allocation from the boot allocator
xen/arm: Move fixmap definitions in a separate header
xen/arm: mm: Clean-up the includes and order them
xen/arm: mm: Use the PMAP helpers in xen_{,un}map_table()
xen/arm64: mm: Add memory to the boot allocator first
xen/arm: mm: Rework setup_xenheap_mappings()
xen/arm: mm: Re-implement setup_frame_table_mappings() with
map_pages_to_xen()
Wei Liu (1):
xen/arm: add Persistent Map (PMAP) infrastructure
xen/arch/arm/Kconfig| 1 +
xen/arch/arm/acpi/lib.c | 2 +
xen/arch/arm/arm32/head.S | 14 +-
xen/arch/arm/arm64/head.S | 14 +-
xen/arch/arm/include/asm/config.h | 10 +-
xen/arch/arm/include/asm/early_printk.h | 1 +
xen/arch/arm/include/asm/fixmap.h | 41 ++
xen/arch/arm/include/asm/lpae.h | 85 ++--
xen/arch/arm/include/asm/mm.h | 4 -
xen/arch/arm/include/asm/page.h | 8 +
xen/arch/arm/include/asm/pmap.h | 33 ++
xen/arch/arm/kernel.c | 1 +
xen/arch/arm/mm.c | 530 +---
xen/arch/arm/p2m.c | 28 +-
xen/arch/arm/setup.c| 63 ++-
xen/common/Kconfig | 3 +
xen/common/Makefile | 1 +
xen/common/pmap.c | 79
xen/include/xen/acpi.h | 18 +-
xen/include/xen/pmap.h | 16 +
20 files changed, 613 insertions(+), 339 deletions(-)
create mode 100644 xen/arch/arm/include/asm/fixmap.h
create mode 100644 xen/arch/arm/include/asm/pmap.h
create mode 100644 xen/common/pmap.c
create mode 100644 xen/include/xen/pmap.h
--
2.32.0
[PATCH v3 01/19] xen/arm: lpae: Rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS
From: Julien Grall Commit 05031fa87357 "xen/arm: guest_walk: Only generate necessary offsets/masks" introduced LPAE_ENTRIES_MASK_GS. In a follow-up patch, we will use it for to define LPAE_ENTRY_MASK. This will lead to inconsistent naming. As LPAE_ENTRY_MASK is used in many places, it is better to rename LPAE_ENTRIES_MASK_GS and avoid some churn. So rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS. Signed-off-by: Julien Grall --- Changes in v2: - New patch --- xen/arch/arm/include/asm/lpae.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h index e94de2e7d8e8..4fb9a40a4ca9 100644 --- a/xen/arch/arm/include/asm/lpae.h +++ b/xen/arch/arm/include/asm/lpae.h @@ -180,7 +180,7 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level) */ #define LPAE_SHIFT_GS(gs) ((gs) - 3) #define LPAE_ENTRIES_GS(gs) (_AC(1, U) << LPAE_SHIFT_GS(gs)) -#define LPAE_ENTRIES_MASK_GS(gs) (LPAE_ENTRIES_GS(gs) - 1) +#define LPAE_ENTRY_MASK_GS(gs) (LPAE_ENTRIES_GS(gs) - 1) #define LEVEL_ORDER_GS(gs, lvl) ((3 - (lvl)) * LPAE_SHIFT_GS(gs)) #define LEVEL_SHIFT_GS(gs, lvl) (LEVEL_ORDER_GS(gs, lvl) + (gs)) @@ -188,7 +188,7 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level) /* Offset in the table at level 'lvl' */ #define LPAE_TABLE_INDEX_GS(gs, lvl, addr) \ -(((addr) >> LEVEL_SHIFT_GS(gs, lvl)) & LPAE_ENTRIES_MASK_GS(gs)) +(((addr) >> LEVEL_SHIFT_GS(gs, lvl)) & LPAE_ENTRY_MASK_GS(gs)) /* Generate an array @var containing the offset for each level from @addr */ #define DECLARE_OFFSETS(var, addr) \ -- 2.32.0
[PATCH 2/3] xen: Rename asprintf() to xasprintf()
Coverity reports that there is a memory leak in
ioreq_server_alloc_rangesets(). This would be true if Xen's implementation of
asprintf() had glibc's return semantics, but it doesn't.
Rename to xasprintf() to reduce confusion for Coverity and other developers.
While at it, fix style issues. Rearrange ioreq_server_alloc_rangesets() to
use a tabulated switch statement, and not to have a trailing space in the
rangeset name for an unknown range type.
Coverity-ID: 1472735
Coverity-ID: 1500265
Fixes: 780e918a2e54 ("add an implentation of asprintf() for xen")
Signed-off-by: Andrew Cooper
---
CC: George Dunlap
CC: Ian Jackson
CC: Jan Beulich
CC: Stefano Stabellini
CC: Wei Liu
CC: Julien Grall
CC: Paul Durrant
---
xen/common/ioreq.c| 16 ++--
xen/common/vsprintf.c | 11 ++-
xen/include/xen/lib.h | 4 ++--
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index 689d256544c8..5c94e74293ce 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -501,13 +501,17 @@ static int ioreq_server_alloc_rangesets(struct
ioreq_server *s,
for ( i = 0; i < NR_IO_RANGE_TYPES; i++ )
{
-char *name;
+char *name, *type;
-rc = asprintf(&name, "ioreq_server %d %s", id,
- (i == XEN_DMOP_IO_RANGE_PORT) ? "port" :
- (i == XEN_DMOP_IO_RANGE_MEMORY) ? "memory" :
- (i == XEN_DMOP_IO_RANGE_PCI) ? "pci" :
- "");
+switch ( i )
+{
+case XEN_DMOP_IO_RANGE_PORT: type = " port"; break;
+case XEN_DMOP_IO_RANGE_MEMORY: type = " memory"; break;
+case XEN_DMOP_IO_RANGE_PCI:type = " pci";break;
+default: type = "";break;
+}
+
+rc = xasprintf(&name, "ioreq_server %d%s", id, type);
if ( rc )
goto fail;
diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index 185a4bd5610a..b278961cc387 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -859,7 +859,7 @@ int scnprintf(char * buf, size_t size, const char *fmt, ...)
EXPORT_SYMBOL(scnprintf);
/**
- * vasprintf - Format a string and allocate a buffer to place it in
+ * xvasprintf - Format a string and allocate a buffer to place it in
*
* @bufp: Pointer to a pointer to receive the allocated buffer
* @fmt: The format string to use
@@ -870,7 +870,7 @@ EXPORT_SYMBOL(scnprintf);
* guaranteed to be null terminated. The memory is allocated
* from xenheap, so the buffer should be freed with xfree().
*/
-int vasprintf(char **bufp, const char *fmt, va_list args)
+int xvasprintf(char **bufp, const char *fmt, va_list args)
{
va_list args_copy;
size_t size;
@@ -891,7 +891,7 @@ int vasprintf(char **bufp, const char *fmt, va_list args)
}
/**
- * asprintf - Format a string and place it in a buffer
+ * xasprintf - Format a string and place it in a buffer
* @bufp: Pointer to a pointer to receive the allocated buffer
* @fmt: The format string to use
* @...: Arguments for the format string
@@ -901,14 +901,15 @@ int vasprintf(char **bufp, const char *fmt, va_list args)
* guaranteed to be null terminated. The memory is allocated
* from xenheap, so the buffer should be freed with xfree().
*/
-int asprintf(char **bufp, const char *fmt, ...)
+int xasprintf(char **bufp, const char *fmt, ...)
{
va_list args;
int i;
va_start(args, fmt);
-i=vasprintf(bufp,fmt,args);
+i = xvasprintf(bufp, fmt, args);
va_end(args);
+
return i;
}
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index c6987973bf88..aea60d292724 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -158,9 +158,9 @@ extern int scnprintf(char * buf, size_t size, const char *
fmt, ...)
__attribute__ ((format (printf, 3, 4)));
extern int vscnprintf(char *buf, size_t size, const char *fmt, va_list args)
__attribute__ ((format (printf, 3, 0)));
-extern int asprintf(char ** bufp, const char * fmt, ...)
+extern int xasprintf(char **bufp, const char *fmt, ...)
__attribute__ ((format (printf, 2, 3)));
-extern int vasprintf(char ** bufp, const char * fmt, va_list args)
+extern int xvasprintf(char **bufp, const char *fmt, va_list args)
__attribute__ ((format (printf, 2, 0)));
long simple_strtol(
--
2.11.0
[PATCH 1/3] tests/resource: Initialise gnttab before xenforeignmemory_map_resource()
It the 'addr' input to mmap(), and currently consuming stack rubble.
Coverity-ID: 1500115
Fixes: c7a7f14b9299 ("tests/resource: Extend to check that the grant frames are
mapped correctly")
Signed-off-by: Andrew Cooper
---
CC: Roger Pau Monné
---
tools/tests/resource/test-resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/tests/resource/test-resource.c
b/tools/tests/resource/test-resource.c
index 0557f8a1b585..189353ebcb43 100644
--- a/tools/tests/resource/test-resource.c
+++ b/tools/tests/resource/test-resource.c
@@ -24,7 +24,7 @@ static void test_gnttab(uint32_t domid, unsigned int
nr_frames,
unsigned long gfn)
{
xenforeignmemory_resource_handle *res;
-grant_entry_v1_t *gnttab;
+grant_entry_v1_t *gnttab = NULL;
size_t size;
int rc;
uint32_t refs[nr_frames], domids[nr_frames];
--
2.11.0
[PATCH 3/3] CI: Coverity tweaks
* Use workflow_dispatch to allow manual creation of the job.
* Use parallel builds. The workers have two vCPUs.
* Shrink the dependency list further. build-essential covers make and gcc,
while bridge-utils and iproute2 are runtime dependencies not build
dependencies. Alter bzip2 to libbz2-dev.
Signed-off-by: Andrew Cooper
---
CC: Roger Pau Monné
---
.github/workflows/coverity.yml | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 9d04b56fd31d..6e7b81e74f72 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -2,6 +2,7 @@ name: Coverity Scan
# We only want to test official release code, not every pull request.
on:
+ workflow_dispatch:
schedule:
- cron: '18 9 * * WED,SUN' # Bi-weekly at 9:18 UTC
@@ -11,11 +12,11 @@ jobs:
steps:
- name: Install build dependencies
run: |
-sudo apt-get install -y wget git gawk bridge-utils \
- iproute2 bzip2 build-essential \
- make gcc zlib1g-dev libncurses5-dev iasl \
- libbz2-dev e2fslibs-dev git-core uuid-dev ocaml \
- ocaml-findlib xz-utils libyajl-dev \
+sudo apt-get install -y wget git gawk \
+ libbz2-dev build-essential \
+ zlib1g-dev libncurses5-dev iasl \
+ libbz2-dev e2fslibs-dev uuid-dev ocaml \
+ ocaml-findlib libyajl-dev \
autoconf libtool liblzma-dev \
python3-dev golang python-dev libsystemd-dev
@@ -31,7 +32,7 @@ jobs:
- name: Pre build stuff
run: |
-make mini-os-dir
+make -j`nproc` mini-os-dir
- uses: vapier/coverity-scan-action@v1
with:
@@ -39,3 +40,4 @@ jobs:
project: XenProject
email: ${{ secrets.COVERITY_SCAN_EMAIL }}
token: ${{ secrets.COVERITY_SCAN_TOKEN }}
+command: make -j`nproc` build
--
2.11.0
[PATCH 0/3] Misc coverity fixes and tweaks
Andrew Cooper (3): tests/resource: Initialise gnttab before xenforeignmemory_map_resource() xen: Rename asprintf() to xasprintf() CI: Coverity tweaks .github/workflows/coverity.yml | 14 -- tools/tests/resource/test-resource.c | 2 +- xen/common/ioreq.c | 16 ++-- xen/common/vsprintf.c| 11 ++- xen/include/xen/lib.h| 4 ++-- 5 files changed, 27 insertions(+), 20 deletions(-) -- 2.11.0
[PATCH] coverity: disable flight from crontab
We are currently doing the Coverity Scans using a github workflow. Signed-off-by: Roger Pau Monné --- crontab | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crontab b/crontab index 8d9b31f1..f43e8bc5 100755 --- a/crontab +++ b/crontab @@ -12,7 +12,7 @@ MAILTO=osstest-ad...@xenproject.org 0 * * * * cd testing.git && BRANCHES=xen-unstable-smoke ./cr-for-branches branches -q "./cr-daily-branch --real" 4-59/30* * * * cd testing.git && ./cr-for-branches branches -q "./cr-daily-branch --real" 18 9 * * 1,3,5 cd testing.git && BRANCHES='linux-next freebsd-master' ./cr-for-branches branches -w "./cr-daily-branch --real" -18 9 * * 3,7 cd testing.git && BRANCHES=xen-unstable-coverity ./cr-for-branches branches -w "./cr-daily-branch --real" +#189 * * 3,7 cd testing.git && BRANCHES=xen-unstable-coverity ./cr-for-branches branches -w "./cr-daily-branch --real" 34 15 23 * * cd testing.git && BRANCHES=examine ./cr-for-branches branches -w "./cr-daily-branch --real" 18 4 * * * cd testing.git && BRANCHES='linux-3.0 libvirt' ./cr-for-branches branches -w "./cr-daily-branch --real" 6-59/15* * * * cd testing.git && EXTRA_BRANCHES='xen-unstable-smoke linux-3.0 libvirt freebsd-master' ./cr-for-branches bisects -w "./cr-try-bisect --real" -- 2.34.1
Re: [PATCH v2 1/5] x86/cpuid: add CPUID flag for Extended Destination ID support
On Sat, Feb 19, 2022 at 11:24:25AM -, David Woodhouse wrote: > > > > /* > > * With interrupt format set to 0 (non-remappable) bits 55:49 from the > > * IO-APIC RTE and bits 11:5 from the MSI address can be used to store > > * high bits for the Destination ID. This expands the Destination ID > > * field from 8 to 15 bits, allowing to target APIC IDs up 32768. > > */ > > I am not keen on that wording because it doesn't seem to fully reflect the > fact that the I/OAPIC is just a device to turn line interrupts into MSIs. But that's an architecture implementation detail, I'm not sure I've seen this written down explicitly in any specification. > The values in bits 55:49 of the RTE *are* what go into bits 11:5 of the > resulting MSI address. Perhaps make it more parenthetical to make it > clearer that they are not independent... "bits 11:5 of the MSI address > (which come from bits 55:49 of the I/OAPIC RTE)..." That could be an option also, as long as it's clear to which bits of the IO-APIC RTE this affects. Thanks, Roger.
