[OXT-250] Update Xen from 4.3.3 to 4.3.4, rolling back some hand rolled XSA patche...

[brendank310]

...s to use upstreams latest.

Signed-off-by: Brendan Kerrigan [email protected]

[brendank310]

Required:
OpenXT/openxt#56

[rianquinn]

Do we know why this is removed?

[brendank310]

It is already defined in another header.

[cjp256]

LGTM. Installed build derived from autobuilder, boots OK here:

---

\ / /___ _ __ | || | |___ /| || | __ _____
\ // _ \ '_ \ | || |_ |_ | || |_ \ / / |
/ \ / | | | | _| ) | _|> < (
//_|| || ||()__() || //___|

(XEN) Xen version 4.3.4-xc

[cjp256]

Rebased on master.

[brendank310]

As it stands this can't be merged. The xc-xt-v4v.patch doesn't apply correctly to hvm.c. The changes to the hvm_hypercall64 table and hvm_hypercall32 table are too similar, and patch seems to ignore the second hunk. I had done testing on 64 bit HVM so it didn't expose the problem. If I break those two hunks into separate patches they work just fine, but it's an ugly solution. Playing with fuzz options didn't yield anything, and I'm not sure how well that'd play with bitbake anyhow. Anyone have any ideas?

[rianquinn]

Have we tried applying the modifications using quilt? It might generate a more sane patch.

[rossphilipson]

Wow that is crazy. The patch is correct, the offsets are correct. Actually that is scary.

[ktemkin]

The offsets are wrong in the unified diff header for that patch; As a result, patch is viewing everything after the first close-brace as "trailing garbage" and skipping it.

(It's not just the line numbers that are wrong-- but the length of the changed hunks: the parts that follow the commas after the "start" and "end lines. Our fuzzing options would let the line numbers slide, but won't compensate for a header that misreports the length of its code changes/context.)

Running the patch manually with --verbose yields:

checking file xen/arch/x86/hvm/hvm.c
Using Plan A... 
Hmm...  Ignoring the trailing garbage
done

This is an unfortunate result of a patch "feature" in which it ignores anything it doesn't recognize at the start or end of a patchset-- the same feature that lets us use the output of git format-patch directly as patches.

Fixing the start/end indicators makes the patch apply correctly.

index ba57e5a..f2073e9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3365,8 +3365,9 @@
     HYPERCALL(hvm_op),
     HYPERCALL(sysctl),
     HYPERCALL(domctl),
     HYPERCALL(tmem_op),
+    HYPERCALL(v4v_op),
     [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
 };

 #define COMPAT_CALL(x)                                        \
@@ -3385,8 +3386,9 @@
     HYPERCALL(hvm_op),
     HYPERCALL(sysctl),
     HYPERCALL(domctl),
     HYPERCALL(tmem_op),
+    HYPERCALL(v4v_op),
     [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
 };

 int hvm_do_hypercall(struct cpu_user_regs *regs)

Was this patch modified by hand? It'd be a pretty bad bug if some of our software was generating malformed patches.

[brendank310]

Yes (and no). As committed yes it was modified by hand. After I saw that behavior I went down the diff -u route. Oddly enough, it still wouldn't apply to the second hypercall table. I even went so far as adding additional context (-U ) with no luck.

Is there anything we can do to enforce some strict type of patch application? It bothers me that this failed silently.

[ktemkin]

Unfortunately, there's no easy way that I know of: patch always allows "garbage" to be stripped without more than that note. Even if we could somehow error specifically on trailing garbage, that would break compatibility with a lot of our existing patches: many of them are generated with trailing garbage by git.

We might be able to come up with some kind of heuristic method to error out on these (e.g. error out if the number of hunks applied don't match the number of unified diff @@ xx @@ headers), but I can't think of anything as easy as adding a command-line option to our patch.

I think I'd chalk this up to a downside of patch-queues. =(

[ktemkin]

+1.

With the corrected patch, I was able to boot a 32-bit and 64-bit guest and verify basic functionality (including Xenstore reads from inside of Windows 32-bit guest).

(Here's an ISO with the 4.3.4 upgrade, in case anyone wants to test: http://dev.ktemkin.com/openxt/xen_4_3_4_openxt.iso)

[rossphilipson]

This was tested last week and Kyle fixed the patch issue and tested over the weekend. Looks good, merging.