Hi, On 27/09/2019 13:11, Volodymyr Babchuk wrote:
Julien Grall writes:Using alternative_if makes the code a bit more streamlined. Take the opportunity to use the new auto-nop infrastructure to avoid counting the number of nop in the else part for arch/arm/arm64/entry.S Signed-off-by: Julien Grall <julien.gr...@arm.com> --- This is pretty much a matter of taste, but at least for arm64 this allows us to use the auto-nop infrastructure. So the arm32 is more to keep inline with arm64. --- xen/arch/arm/arm32/entry.S | 9 ++++++--- xen/arch/arm/arm64/entry.S | 8 +++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S index 0b4cd19abd..1428cd3583 100644 --- a/xen/arch/arm/arm32/entry.S +++ b/xen/arch/arm/arm32/entry.S @@ -65,9 +65,12 @@ save_guest_regs: * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu * feature, the checking of pending SErrors will be skipped. */ - ALTERNATIVE("nop", - "b skip_check", - SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT) + alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT + nop + alternative_else + b skip_check + alternative_endif +for the arm32 code you can have my r-b: Reviewed-By: Volodymyr Babchuk <volodymyr_babc...@epam.com>/* * Start to check pending virtual abort in the gap of Guest -> HYP * world switch. diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S index 458d12f188..91cf6ee6f4 100644 --- a/xen/arch/arm/arm64/entry.S +++ b/xen/arch/arm/arm64/entry.S @@ -170,9 +170,11 @@ * is not set. If a vSError took place, the initial exception will be * skipped. Exit ASAP */ - ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f", - "nop; nop", - SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT) + alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT + bl check_pending_vserror + cbnz x0, 1f + alternative_else_nop_endif +You asked other people to do not introduce new code in one patch and rewrite it in the following patch. But there you are doing exactly the same.
This is a fairly borderline comment knowing that I usually don't request clean-up and code consolidation in the same patch.
I believe, it is possible to move all "alternative" patches to the very beginning of the patch series and only then introduce macro guest_vector.
For a first, the first patch is definitely not new code. This is code consolidation and therefore I don't tend to mix the two for clarity. So this should have been a patch before the first patch.
Secondly, the first 4 patches are candidate for backport. The rest of the series would be good to backport but I am not aware of a critical issue in previous Xen release to strongly push for it.
Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
