Re: [Xen-devel] [PATCH 5/6] xen/tasklet: Return -ERESTART from continue_hypercall_on_cpu()
On 10.12.2019 18:55, Andrew Cooper wrote: > On 10/12/2019 08:55, Jan Beulich wrote: >> On 09.12.2019 18:49, Andrew Cooper wrote: >>> On 09/12/2019 16:52, Jan Beulich wrote: On 05.12.2019 23:30, Andrew Cooper wrote: > Some hypercalls tasklets want to create a continuation, rather than fail > the > hypercall with a hard error. By the time the tasklet is executing, it is > too > late to create the continuation, and even continue_hypercall_on_cpu() > doesn't > have enough state to do it correctly. I think it would be quite nice if you made clear what piece of state it is actually missing. To be honest, I don't recall anymore. >>> How to correctly mutate the registers and/or memory (which is specific >>> to the hypercall subop in some cases). >> Well, in-memory arguments can be accessed as long as the mapping is >> the right one (which it typically wouldn't be inside a tasklet). Do >> existing continue_hypercall_on_cpu() users need this? Looking over >> patch 4 again, I didn't think so. (Which isn't to say that removing >> the latent issue is not a good thing.) >> >> In-register values can be changed as long as the respective exit >> path will suitably pick up the value, which I thought was always >> the case. >> >> Hence I'm afraid your single reply sentence didn't really clarify >> matters. I'm sorry if this is just because of me being dense. > > How, physically, would you arrange for continue_hypercall_on_cpu() to > make the requisite state adjustments? You can't (at least not without having sufficient further context), I agree. Yet ... > Yes - registers and memory can be accessed, but only the hypercall > (sub?)op handler knows how to mutate them appropriately. > > You'd have to copy the mutation logic into continue_hypercall_on_cpu(), > and pass in op/subops and a union of all pointers, *and* whatever > intermediate state the subop handler needs. > > Or you can return -ERESTART and let the caller DTRT with the state it > has in context, as it would in other cases requiring a continuation. ... it continues to be unclear to me whether you're fixing an actual bug here, or just a latent one. The existing uses of continue_hypercall_on_cpu() don't look to require state updates beyond the hypercall return value (or else, aiui, they wouldn't have worked in the first place), and that one had a way to get modified. > The current behaviour with this patch is to not cancel the continuation, > which > I think is less bad, but still not great. Thoughts? Well, that's a guest live lock then aiui. >>> It simply continues again. It will livelock only if the hypercall picks >>> a bad cpu all the time. >> Oh, I see I was mislead by continue_hypercall_tasklet_handler() not >> updating info->cpu, not paying attention to it actually freeing info. >> Plus a crucial aspect looks to be that there are no "chained" uses of >> continue_hypercall_on_cpu() anymore (the microcode loading one being >> gone now) - afaict any such wouldn't guarantee forward progress with >> this new model (without recording somewhere which CPUs had been dealt >> with already). > > I'd forgotten that we had that, but I can't say I'm sad to see the back > of it. I recall at the time saying that it wasn't a clever move. > > For now, I suggest that we ignore that case. If an when a real usecase > appears, we can consider making adjustments. Oh, of course - I didn't mean to even remotely suggest anything else. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] clock source in PV Linux
Jürgen, Boris, I've noticed <6>clocksource: Switched to clocksource tsc as the final clocksource related boot message in a PV Dom0's log with 5.4.2. Is it intentional that it's not the "xen" one that gets used by default? Thanks, Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.13-testing test] 144673: tolerable FAIL - PUSHED
flight 144673 xen-4.13-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/144673/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail REGR. vs. 144609 test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 144609 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen b0f0bbca95bd532212fb1956f3e23d1ab13a53cf baseline version: xen d7abfd2c4b6eb43297efd648238aa426a1ab117b Last test of basis 144609 2019-12-06 19:06:05 Z4 days Failing since144640 2019-12-09 14:36:33 Z1 days4 attempts Testing same since 144673 2019-12-10 19:07:50 Z0 days1 attempts People who touched revisions under
[Xen-devel] [ovmf test] 144683: regressions - FAIL
flight 144683 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/144683/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 144637 build-i386-xsm6 xen-buildfail REGR. vs. 144637 build-amd64-xsm 6 xen-buildfail REGR. vs. 144637 build-i3866 xen-buildfail REGR. vs. 144637 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a version targeted for testing: ovmf 7e55cf6b48dcd43de46d008b2f12caaad2554503 baseline version: ovmf 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 Last test of basis 144637 2019-12-09 09:09:49 Z1 days Failing since144646 2019-12-10 01:39:53 Z1 days5 attempts Testing same since 144661 2019-12-10 12:09:18 Z0 days3 attempts People who touched revisions under test: Bob Feng Jiewen Yao Steven Shi jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 Not pushing. commit 7e55cf6b48dcd43de46d008b2f12caaad2554503 Author: Jiewen Yao Date: Sat Dec 7 21:41:10 2019 +0800 SecurityPkg/Tcg2Smm: Measure the table before patch. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1940 According to TCG PFP specification: the ACPI table must be measured prior to any modification, and the measurement must be same cross every boot cycle. There is a fix 3a63c17ebc853cbb27d190729d01e27f68e65b94 for the HID data. However that is not enough. The LAML/LASA and PCD configuration change may also cause similar problem. We need measure the table before any update. Cc: Jian J Wang Cc: Chao Zhang Signed-off-by: Jiewen Yao Reviewed-by: Chao Zhang commit a80032dc44a1071a34f4415a7c5cef5170ee6159 Author: Steven Shi Date: Tue Nov 19 16:22:08 2019 +0800 BaseTools: Remove redundant binary cache file Redesign the binary cache and not need to save the cache intermediate result and state in memory as a ModuleBuildCacheIR class instance. So remove the CacheIR.py which define the ModuleBuildCacheIR class. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit fc8b8deac2d77524ff8cfe44acf95b5e1f59804e Author: Steven Shi Date: Tue Nov 19 16:17:00 2019 +0800 BaseTools: Leverage compiler output to optimize binary cache Redesign the binary cache and bases on the compiler to output the dependency header files info for every module. The binary cache will directly consume the dependency header files info and doesn't parse the C source code by iteself. Also redesign the dependency files list format for module and try to share the common lib hash result as more as possible in local process. Remove the unnecessary share data access across multiprocessing. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit 3bfbc915074a45f4d9c61aa2b698a62f1a24124e Author: Steven Shi Date: Mon Oct 21 14:51:49 2019 +0800 BaseTools: enhance the CacheCopyFile method arg names Enhance the CacheCopyFile method arg names to be
[Xen-devel] [PATCH v6 2/3] xen/blkback: Squeeze page pools if a memory pressure is detected
Each `blkif` has a free pages pool for the grant mapping. The size of
the pool starts from zero and be increased on demand while processing
the I/O requests. If current I/O requests handling is finished or 100
milliseconds has passed since last I/O requests handling, it checks and
shrinks the pool to not exceed the size limit, `max_buffer_pages`.
Therefore, if a system (maybe mistakenly) allows `blkfront` running
guests to attach a large number of devices, the guests could cause a
memory pressure in the `blkback` running guest by attaching a large
number of block devices and inducing I/O. System administrators can
avoid such problematic situations by limiting the maximum number of
devices that can be attached, but finding the optimal limit is not so
easy. Improper set of the limit can results in the memory pressure or a
resource underutilization. This commit avoids such problematic
situations by squeezing the pools (returns every free page in the pool
to the system) for a while (users can set this duration via a module
parameter) if a memory pressure is detected.
Discussions
===
The `blkback`'s original shrinking mechanism returns only pages in the
pool, which are not currently be used by `blkback`, to the system. In
other words, the pages that are not mapped with granted pages. Because
this commit is changing only the shrink limit but still uses the same
freeing mechanism it does not touch pages which are currently mapping
grants.
Once a memory pressure is detected, this commit keeps the squeezing
limit for a user-specified time duration. The duration should be
neither too long nor too short. If it is too long, the squeezing
incurring overhead can reduce the I/O performance. If it is too short,
`blkback` will not free enough pages to reduce the memory pressure.
This commit sets the value as `10 milliseconds` by default because it is
a short time in terms of I/O while it is a long time in terms of memory
operations. Also, as the original shrinking mechanism works for at
least every 100 milliseconds, this could be a somewhat reasonable
choice. I also tested other durations (refer to the below section for
more details) and confirmed that 10 milliseconds is the one that works
best with the test. That said, the proper duration depends on actual
configurations and workloads. That's why this commit allows users to
set the duration as a module parameter.
Memory Pressure Test
To show how this commit fixes the memory pressure situation well, I
configured a test environment on a xen-running virtualization system.
On the `blkfront` running guest instances, I attach a large number of
network-backed volume devices and induce I/O to those. Meanwhile, I
measure the number of pages that swapped in (pswpin) and out (pswpout)
on the `blkback` running guest. The test ran twice, once for the
`blkback` before this commit and once for that after this commit. As
shown below, this commit has dramatically reduced the memory pressure:
pswpin pswpout
before 76,672 185,799
after 2123,325
Optimal Aggressive Shrinking Duration
-
To find a best squeezing duration, I repeated the test with three
different durations (1ms, 10ms, and 100ms). The results are as below:
durationpswpin pswpout
1 852 6,424
10 212 3,325
100 203 3,340
As expected, the memory pressure has decreased as the duration is
increased, but the reduction stopped from the `10ms`. Based on this
results, I chose the default duration as 10ms.
Performance Overhead Test
=
This commit could incur I/O performance degradation under severe memory
pressure because the squeezing will require more page allocations per
I/O. To show the overhead, I artificially made a worst-case squeezing
situation and measured the I/O performance of a `blkfront` running
guest.
For the artificial squeezing, I set the `blkback.max_buffer_pages` using
the `/sys/module/xen_blkback/parameters/max_buffer_pages` file. We set
the value to `1024` and `0`. The `1024` is the default value. Setting
the value as `0` is same to a situation doing the squeezing always
(worst-case).
For the I/O performance measurement, I use a simple `dd` command.
Default Performance
---
[dom0]# echo 1024 > /sys/module/xen_blkback/parameters/max_buffer_pages
[instance]$ for i in {1..5}; do dd if=/dev/zero of=file \
bs=4k count=$((256*512)); sync; done
131072+0 records in
131072+0 records out
536870912 bytes (537 MB) copied, 11.7257 s, 45.8 MB/s
131072+0 records in
131072+0 records out
536870912 bytes (537 MB) copied, 13.8827 s, 38.7 MB/s
131072+0 records in
131072+0 records out
536870912 bytes (537 MB) copied, 13.8781 s, 38.7 MB/s
131072+0 records in
131072+0 records out
536870912 bytes (537 MB)
[Xen-devel] [PATCH v6 3/3] xen/blkback: Remove unnecessary static variable name prefixes
A few of static variables in blkback have 'xen_blkif_' prefix, though it
is unnecessary for static variables. This commit removes such prefixes.
Signed-off-by: SeongJae Park
---
drivers/block/xen-blkback/blkback.c | 37 +
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/drivers/block/xen-blkback/blkback.c
b/drivers/block/xen-blkback/blkback.c
index b493c306e84f..f690373669b8 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -62,8 +62,8 @@
* IO workloads.
*/
-static int xen_blkif_max_buffer_pages = 1024;
-module_param_named(max_buffer_pages, xen_blkif_max_buffer_pages, int, 0644);
+static int max_buffer_pages = 1024;
+module_param_named(max_buffer_pages, max_buffer_pages, int, 0644);
MODULE_PARM_DESC(max_buffer_pages,
"Maximum number of free pages to keep in each block backend buffer");
@@ -78,8 +78,8 @@ MODULE_PARM_DESC(max_buffer_pages,
* algorithm.
*/
-static int xen_blkif_max_pgrants = 1056;
-module_param_named(max_persistent_grants, xen_blkif_max_pgrants, int, 0644);
+static int max_pgrants = 1056;
+module_param_named(max_persistent_grants, max_pgrants, int, 0644);
MODULE_PARM_DESC(max_persistent_grants,
"Maximum number of grants to map persistently");
@@ -88,8 +88,8 @@ MODULE_PARM_DESC(max_persistent_grants,
* use. The time is in seconds, 0 means indefinitely long.
*/
-static unsigned int xen_blkif_pgrant_timeout = 60;
-module_param_named(persistent_grant_unused_seconds, xen_blkif_pgrant_timeout,
+static unsigned int pgrant_timeout = 60;
+module_param_named(persistent_grant_unused_seconds, pgrant_timeout,
uint, 0644);
MODULE_PARM_DESC(persistent_grant_unused_seconds,
"Time in seconds an unused persistent grant is allowed to "
@@ -137,9 +137,8 @@ module_param(log_stats, int, 0644);
static inline bool persistent_gnt_timeout(struct persistent_gnt
*persistent_gnt)
{
- return xen_blkif_pgrant_timeout &&
- (jiffies - persistent_gnt->last_used >=
- HZ * xen_blkif_pgrant_timeout);
+ return pgrant_timeout && (jiffies - persistent_gnt->last_used >=
+ HZ * pgrant_timeout);
}
/* Once a memory pressure is detected, squeeze free page pools for a while. */
@@ -249,7 +248,7 @@ static int add_persistent_gnt(struct xen_blkif_ring *ring,
struct persistent_gnt *this;
struct xen_blkif *blkif = ring->blkif;
- if (ring->persistent_gnt_c >= xen_blkif_max_pgrants) {
+ if (ring->persistent_gnt_c >= max_pgrants) {
if (!blkif->vbd.overflow_max_grants)
blkif->vbd.overflow_max_grants = 1;
return -EBUSY;
@@ -412,14 +411,13 @@ static void purge_persistent_gnt(struct xen_blkif_ring
*ring)
goto out;
}
- if (ring->persistent_gnt_c < xen_blkif_max_pgrants ||
- (ring->persistent_gnt_c == xen_blkif_max_pgrants &&
+ if (ring->persistent_gnt_c < max_pgrants ||
+ (ring->persistent_gnt_c == max_pgrants &&
!ring->blkif->vbd.overflow_max_grants)) {
num_clean = 0;
} else {
- num_clean = (xen_blkif_max_pgrants / 100) * LRU_PERCENT_CLEAN;
- num_clean = ring->persistent_gnt_c - xen_blkif_max_pgrants +
- num_clean;
+ num_clean = (max_pgrants / 100) * LRU_PERCENT_CLEAN;
+ num_clean = ring->persistent_gnt_c - max_pgrants + num_clean;
num_clean = min(ring->persistent_gnt_c, num_clean);
pr_debug("Going to purge at least %u persistent grants\n",
num_clean);
@@ -614,8 +612,7 @@ static void print_stats(struct xen_blkif_ring *ring)
current->comm, ring->st_oo_req,
ring->st_rd_req, ring->st_wr_req,
ring->st_f_req, ring->st_ds_req,
-ring->persistent_gnt_c,
-xen_blkif_max_pgrants);
+ring->persistent_gnt_c, max_pgrants);
ring->st_print = jiffies + msecs_to_jiffies(10 * 1000);
ring->st_rd_req = 0;
ring->st_wr_req = 0;
@@ -675,7 +672,7 @@ int xen_blkif_schedule(void *arg)
if (time_before(jiffies, buffer_squeeze_end))
shrink_free_pagepool(ring, 0);
else
- shrink_free_pagepool(ring, xen_blkif_max_buffer_pages);
+ shrink_free_pagepool(ring, max_buffer_pages);
if (log_stats && time_after(jiffies, ring->st_print))
print_stats(ring);
@@ -902,7 +899,7 @@ static int xen_blkbk_map(struct xen_blkif_ring *ring,
continue;
}
if (use_persistent_gnts &&
- ring->persistent_gnt_c < xen_blkif_max_pgrants) {
+ ring->persistent_gnt_c < max_pgrants) {
/*
[Xen-devel] [PATCH v6 1/3] xenbus/backend: Add memory pressure handler callback
Granting pages consumes backend system memory. In systems configured
with insufficient spare memory for those pages, it can cause a memory
pressure situation. However, finding the optimal amount of the spare
memory is challenging for large systems having dynamic resource
utilization patterns. Also, such a static configuration might lack
flexibility.
To mitigate such problems, this commit adds a memory reclaim callback to
'xenbus_driver'. If a memory pressure is detected, 'xenbus' requests
every backend driver to volunarily release its memory.
Note that it would be able to improve the callback facility for more
sophisticated handlings of general pressures. For example, it would be
possible to monitor the memory consumption of each device and issue the
release requests to only devices which causing the pressure. Also, the
callback could be extended to handle not only memory, but general
resources. Nevertheless, this version of the implementation defers such
sophisticated goals as a future work.
Reviewed-by: Juergen Gross
Signed-off-by: SeongJae Park
---
drivers/xen/xenbus/xenbus_probe_backend.c | 32 +++
include/xen/xenbus.h | 1 +
2 files changed, 33 insertions(+)
diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c
b/drivers/xen/xenbus/xenbus_probe_backend.c
index b0bed4faf44c..aedbe2198de5 100644
--- a/drivers/xen/xenbus/xenbus_probe_backend.c
+++ b/drivers/xen/xenbus/xenbus_probe_backend.c
@@ -248,6 +248,35 @@ static int backend_probe_and_watch(struct notifier_block
*notifier,
return NOTIFY_DONE;
}
+static int xenbus_backend_reclaim(struct device *dev, void *data)
+{
+ struct xenbus_driver *drv;
+
+ if (!dev->driver)
+ return 0;
+ drv = to_xenbus_driver(dev->driver);
+ if (drv && drv->reclaim)
+ drv->reclaim(to_xenbus_device(dev));
+ return 0;
+}
+
+/*
+ * Returns 0 always because we are using shrinker to only detect memory
+ * pressure.
+ */
+static unsigned long xenbus_backend_shrink_count(struct shrinker *shrinker,
+ struct shrink_control *sc)
+{
+ bus_for_each_dev(_backend.bus, NULL, NULL,
+ xenbus_backend_reclaim);
+ return 0;
+}
+
+static struct shrinker xenbus_backend_shrinker = {
+ .count_objects = xenbus_backend_shrink_count,
+ .seeks = DEFAULT_SEEKS,
+};
+
static int __init xenbus_probe_backend_init(void)
{
static struct notifier_block xenstore_notifier = {
@@ -264,6 +293,9 @@ static int __init xenbus_probe_backend_init(void)
register_xenstore_notifier(_notifier);
+ if (register_shrinker(_backend_shrinker))
+ pr_warn("shrinker registration failed\n");
+
return 0;
}
subsys_initcall(xenbus_probe_backend_init);
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 869c816d5f8c..196260017666 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -104,6 +104,7 @@ struct xenbus_driver {
struct device_driver driver;
int (*read_otherend_details)(struct xenbus_device *dev);
int (*is_ready)(struct xenbus_device *dev);
+ void (*reclaim)(struct xenbus_device *dev);
};
static inline struct xenbus_driver *to_xenbus_driver(struct device_driver *drv)
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v6 0/2] xenbus/backend: Add a memory pressure handler callback
Granting pages consumes backend system memory. In systems configured with insufficient spare memory for those pages, it can cause a memory pressure situation. However, finding the optimal amount of the spare memory is challenging for large systems having dynamic resource utilization patterns. Also, such a static configuration might lack flexibility. To mitigate such problems, this patchset adds a memory reclaim callback to 'xenbus_driver' (patch 1) and use it to mitigate the problem in 'xen-blkback' (patch 2). The third patch is a trivial cleanup of variable names. Base Version This patch is based on v5.4. A complete tree is also available at my public git repo: https://github.com/sjp38/linux/tree/blkback_squeezing_v6 Patch History - Changes from v5 (https://lore.kernel.org/linux-block/20191210080628.5264-1-sjp...@amazon.de/) - Wordsmith the commit messages (suggested by Roger Pau Monné) - Change the reclaim callback return type (suggested by Roger Pau Monné) - Change the type of the blkback squeeze duration variable (suggested by Roger Pau Monné) - Add a patch for removal of unnecessary static variable name prefixes (suggested by Roger Pau Monné) - Fix checkpatch.pl warnings Changes from v4 (https://lore.kernel.org/xen-devel/20191209194305.20828-1-sjp...@amazon.com/) - Remove domain id parameter from the callback (suggested by Juergen Gross) - Rename xen-blkback module parameter (suggested by Stefan Nuernburger) Changes from v3 (https://lore.kernel.org/xen-devel/20191209085839.21215-1-sjp...@amazon.com/) - Add general callback in xen_driver and use it (suggested by Juergen Gross) Changes from v2 (https://lore.kernel.org/linux-block/af195033-23d5-38ed-b73b-f6e2e3b34...@amazon.com) - Rename the module parameter and variables for brevity (aggressive shrinking -> squeezing) Changes from v1 (https://lore.kernel.org/xen-devel/20191204113419.2298-1-sjp...@amazon.com/) - Adjust the description to not use the term, `arbitrarily` (suggested by Paul Durrant) - Specify time unit of the duration in the parameter description, (suggested by Maximilian Heyne) - Change default aggressive shrinking duration from 1ms to 10ms - Merge two patches into one single patch SeongJae Park (2): xenbus/backend: Add memory pressure handler callback xen/blkback: Squeeze page pools if a memory pressure is detected drivers/block/xen-blkback/blkback.c | 23 +++-- drivers/block/xen-blkback/common.h| 1 + drivers/block/xen-blkback/xenbus.c| 3 ++- drivers/xen/xenbus/xenbus_probe_backend.c | 31 +++ include/xen/xenbus.h | 1 + 5 files changed, 56 insertions(+), 3 deletions(-) -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 2/2] xen/blkback: Squeeze page pools if a memory pressure is detected
On Tue, 10 Dec 2019 12:04:32 +0100 "Roger Pau Monné" wrote: > > Each `blkif` has a free pages pool for the grant mapping. The size of > > the pool starts from zero and be increased on demand while processing > > the I/O requests. If current I/O requests handling is finished or 100 > > milliseconds has passed since last I/O requests handling, it checks and > > shrinks the pool to not exceed the size limit, `max_buffer_pages`. > > > > Therefore, `blkfront` running guests can cause a memory pressure in the > > `blkback` running guest by attaching a large number of block devices and > > inducing I/O. > > Hm, I don't think this is actually true. blkfront cannot attach an > arbitrary number of devices, blkfront is just a frontend for a device > that's instantiated by the Xen toolstack, so it's the toolstack the one > that controls the amount of PV block devices. Right, the problem can occur only if it is mis-configured so that the frontend running guests can attach a large number of devices which is enough to cause the memory pressure. I tried to explain it in below paragraph, but seems above paragraph is a little bit confusing. I will wordsmith the sentence in the next version. > > > System administrators can avoid such problematic > > situations by limiting the maximum number of devices each guest can > > attach. However, finding the optimal limit is not so easy. Improper > > set of the limit can results in the memory pressure or a resource > > underutilization. This commit avoids such problematic situations by > > squeezing the pools (returns every free page in the pool to the system) > > for a while (users can set this duration via a module parameter) if a > > memory pressure is detected. > > > > Discussions > > === > > > > The `blkback`'s original shrinking mechanism returns only pages in the > > pool, which are not currently be used by `blkback`, to the system. In > > other words, the pages are not mapped with foreign pages. Because this > ^ that ^ granted > > commit is changing only the shrink limit but uses the mechanism as is, > > this commit does not introduce improper mappings related security > > issues. > > That last sentence is hard to parse. I think something like: > > "Because this commit is changing only the shrink limit but still uses the > same freeing mechanism it does not touch pages which are currently > mapping grants." > > > > > Once a memory pressure is detected, this commit keeps the squeezing > > limit for a user-specified time duration. The duration should be > > neither too long nor too short. If it is too long, the squeezing > > incurring overhead can reduce the I/O performance. If it is too short, > > `blkback` will not free enough pages to reduce the memory pressure. > > This commit sets the value as `10 milliseconds` by default because it is > > a short time in terms of I/O while it is a long time in terms of memory > > operations. Also, as the original shrinking mechanism works for at > > least every 100 milliseconds, this could be a somewhat reasonable > > choice. I also tested other durations (refer to the below section for > > more details) and confirmed that 10 milliseconds is the one that works > > best with the test. That said, the proper duration depends on actual > > configurations and workloads. That's why this commit is allowing users > ^ allows > > to set it as their optimal value via the module parameter. > > ... to set the duration as a module parameter. Thank you for great suggestions, I will apply those. > > > > > Memory Pressure Test > > > > > > To show how this commit fixes the memory pressure situation well, I > > configured a test environment on a xen-running virtualization system. > > On the `blkfront` running guest instances, I attach a large number of > > network-backed volume devices and induce I/O to those. Meanwhile, I > > measure the number of pages that swapped in and out on the `blkback` > > running guest. The test ran twice, once for the `blkback` before this > > commit and once for that after this commit. As shown below, this commit > > has dramatically reduced the memory pressure: > > > > pswpin pswpout > > I assume pswpin means 'pages swapped in' and pswpout 'pages swapped > out'. Might be good to add a note to that effect. Good point! I will add the note. > > > before 76,672 185,799 > > after 2123,325 > > > > Optimal Aggressive Shrinking Duration > > - > > > > To find a best squeezing duration, I repeated the test with three > > different durations (1ms, 10ms, and 100ms). The results are as below: > > > > durationpswpin pswpout > > 1 852 6,424 > > 10 212 3,325 > > 100 203 3,340 > > > > As expected, the memory pressure has decreased as the
[Xen-devel] [ovmf bisection] complete build-i386-xsm
branch xen-unstable xenbranch xen-unstable job build-i386-xsm testid xen-build Tree: ovmf https://github.com/tianocore/edk2.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: ovmf https://github.com/tianocore/edk2.git Bug introduced: 13c5e34a1b8bfedbd10ea038cfcbae5caeab6652 Bug not present: 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/144687/ commit 13c5e34a1b8bfedbd10ea038cfcbae5caeab6652 Author: Bob Feng Date: Mon Dec 2 16:24:19 2019 +0800 BaseTools: Add build option for dependency file generation BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2311 Add /showIncludes for msvc and -MMD -MF $@.deps for GCC and CLANG Remove /MP for msvc since /MP does not work with /showIncludes Signed-off-by: Bob Feng Cc: Liming Gao Cc: Steven Shi Cc: Michael D Kinney Reviewed-by: Liming Gao For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/ovmf/build-i386-xsm.xen-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/ovmf/build-i386-xsm.xen-build --summary-out=tmp/144687.bisection-summary --basis-template=144637 --blessings=real,real-bisect ovmf build-i386-xsm xen-build Searching for failure / basis pass: 144676 fail [host=huxelrebe1] / 144637 [host=elbling1] 144590 [host=chardonnay1] 144583 [host=albana0] 144578 [host=albana0] 144564 [host=albana0] 144527 [host=huxelrebe0] 144524 ok. Failure / basis pass flights: 144676 / 144524 (tree with no url: minios) Tree: ovmf https://github.com/tianocore/edk2.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest 7e55cf6b48dcd43de46d008b2f12caaad2554503 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 Basis pass c9416efeef0d4a0554db01f3fd1cdaede14856d7 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d d7c3e6c9e9dabbba0b8dc0ddb0fc38811ae0915f Generating revisions with ./adhoc-revtuple-generator https://github.com/tianocore/edk2.git#c9416efeef0d4a0554db01f3fd1cdaede14856d7-7e55cf6b48dcd43de46d008b2f12caaad2554503 git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git#933ebad2470a169504799a1d95b8e410bd9847ef-933ebad2470a169504799a1d95b8e410bd9847ef git://xenbits.xen.org/osstest/seabios.git#c9ba5276e3217ac6a1ec772dbebf568ba3a8a5\ 5d-c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d git://xenbits.xen.org/xen.git#d7c3e6c9e9dabbba0b8dc0ddb0fc38811ae0915f-ae25407faaaddf4abe44137ebf0e177a8c8f9858 Loaded 10001 nodes in revision graph Searching for test results: 144524 pass c9416efeef0d4a0554db01f3fd1cdaede14856d7 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d d7c3e6c9e9dabbba0b8dc0ddb0fc38811ae0915f 144527 [host=huxelrebe0] 144578 [host=albana0] 144583 [host=albana0] 144564 [host=albana0] 144590 [host=chardonnay1] 144637 [host=elbling1] 144646 fail 0c3e8e9947a6c13b4327dd11b20acb95441701cf d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144680 pass 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144681 fail 13c5e34a1b8bfedbd10ea038cfcbae5caeab6652 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144676 fail 7e55cf6b48dcd43de46d008b2f12caaad2554503 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144682 pass 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144684 fail 13c5e34a1b8bfedbd10ea038cfcbae5caeab6652 d0d8ad39ecb51cd7497cd524484fe09f50876798
Re: [Xen-devel] [PATCH v5 1/2] xenbus/backend: Add memory pressure handler callback
On Tue, 10 Dec 2019 11:16:35 +0100 "Roger Pau Monné"
wrote:
> > Granting pages consumes backend system memory. In systems configured
> > with insufficient spare memory for those pages, it can cause a memory
> > pressure situation. However, finding the optimal amount of the spare
> > memory is challenging for large systems having dynamic resource
> > utilization patterns. Also, such a static configuration might lack a
>
> s/lack a/lack/
>
> > flexibility.
> >
> > To mitigate such problems, this commit adds a memory reclaim callback to
> > 'xenbus_driver'. Using this facility, 'xenbus' would be able to monitor
> > a memory pressure and request specific devices of specific backend
>
> s/monitor a/monitor/
>
> > drivers which causing the given pressure to voluntarily release its
>
> ...which are causing...
>
> > memory.
> >
> > That said, this commit simply requests every callback registered driver
> > to release its memory for every domain, rather than issueing the
>
> s/issueing/issuing/
>
> > requests to the drivers and the domain in charge. Such things will be
>
> I'm afraid I don't understand the "domain in charge" part of this
> sentence.
>
> > done in a futur. Also, this commit focuses on memory only. However, it
>
> ... done in a future change. Also I think the period after only should
> be removed in order to tie both sentences together.
>
> > would be ablt to be extended for general resources.
>
> s/ablt/able/
>
> >
> > Signed-off-by: SeongJae Park
> > ---
> > drivers/xen/xenbus/xenbus_probe_backend.c | 31 +++
> > include/xen/xenbus.h | 1 +
> > 2 files changed, 32 insertions(+)
> >
> > diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c
> > b/drivers/xen/xenbus/xenbus_probe_backend.c
> > index b0bed4faf44c..5a5ba29e39df 100644
> > --- a/drivers/xen/xenbus/xenbus_probe_backend.c
> > +++ b/drivers/xen/xenbus/xenbus_probe_backend.c
> > @@ -248,6 +248,34 @@ static int backend_probe_and_watch(struct
> > notifier_block *notifier,
> > return NOTIFY_DONE;
> > }
> >
> > +static int xenbus_backend_reclaim(struct device *dev, void *data)
> > +{
> > + struct xenbus_driver *drv;
>
> Newline and const.
>
> > + if (!dev->driver)
> > + return -ENOENT;
> > + drv = to_xenbus_driver(dev->driver);
> > + if (drv && drv->reclaim)
> > + drv->reclaim(to_xenbus_device(dev));
>
> You seem to completely ignore the return of the reclaim hook...
>
> > + return 0;
> > +}
> > +
> > +/*
> > + * Returns 0 always because we are using shrinker to only detect memory
> > + * pressure.
> > + */
> > +static unsigned long xenbus_backend_shrink_count(struct shrinker *shrinker,
> > + struct shrink_control *sc)
> > +{
> > + bus_for_each_dev(_backend.bus, NULL, NULL,
> > + xenbus_backend_reclaim);
> > + return 0;
> > +}
> > +
> > +static struct shrinker xenbus_backend_shrinker = {
> > + .count_objects = xenbus_backend_shrink_count,
> > + .seeks = DEFAULT_SEEKS,
> > +};
> > +
> > static int __init xenbus_probe_backend_init(void)
> > {
> > static struct notifier_block xenstore_notifier = {
> > @@ -264,6 +292,9 @@ static int __init xenbus_probe_backend_init(void)
> >
> > register_xenstore_notifier(_notifier);
> >
> > + if (register_shrinker(_backend_shrinker))
> > + pr_warn("shrinker registration failed\n");
> > +
> > return 0;
> > }
> > subsys_initcall(xenbus_probe_backend_init);
> > diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> > index 869c816d5f8c..cdb075e4182f 100644
> > --- a/include/xen/xenbus.h
> > +++ b/include/xen/xenbus.h
> > @@ -104,6 +104,7 @@ struct xenbus_driver {
> > struct device_driver driver;
> > int (*read_otherend_details)(struct xenbus_device *dev);
> > int (*is_ready)(struct xenbus_device *dev);
> > + unsigned (*reclaim)(struct xenbus_device *dev);
>
> ... hence I wonder why it's returning an unsigned when it's just
> ignored.
>
> IMO it should return an int to signal errors, and the return should be
> ignored.
I first thought similarly and set the callback to return something. However,
as this callback is called to simply notify the memory pressure and ask the
driver to free its memory as many as possible, I couldn't easily imagine what
kind of errors that need to be handled by its caller can occur in the callback,
especially because current blkback's callback implementation has no such error.
So, if you and others agree, I would like to simply set the return type to
'void' for now and defer the error handling to a future change.
>
> Also, I think it would preferable for this function to take an extra
> parameter to describe the resource the driver should attempt to free
> (ie: memory or interrupts for example). I'm however not able to find
> any existing Linux type to describe such resources.
Yes, such extention would be the right direction. However, because there is no
existing Linux
[Xen-devel] [qemu-mainline test] 144671: tolerable FAIL - PUSHED
flight 144671 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/144671/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 144591 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 15 guest-saverestorefail like 144591 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144591 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 144591 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144591 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 144591 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144591 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass version targeted for testing: qemuu52901abf94477b400cf88c1f70bb305e690ba2de baseline version: qemuu02f9c885edefae66d787847758d13ed60c0f539e Last test of basis 144591 2019-12-06 16:36:24 Z4 days Failing since144638 2019-12-09 13:36:22 Z1 days4 attempts Testing same since 144671 2019-12-10 17:37:17 Z0 days1 attempts People who touched revisions under test: Alexey Kardashevskiy David Gibson Eric Blake Peter Maydell Vladimir Sementsov-Ogievskiy jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm
[Xen-devel] [xen-unstable test] 144668: tolerable FAIL - PUSHED
flight 144668 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/144668/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 144631 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144631 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 144631 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 144631 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail like 144635 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144635 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 144635 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 144635 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144635 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 144635 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen 4935a5433db28077fe6313f920bbedcd54516cec baseline version: xen ae25407faaaddf4abe44137ebf0e177a8c8f9858 Last test of basis 144635 2019-12-09 01:51:59 Z1 days Failing since144641 2019-12-09 17:06:42 Z1 days3 attempts Testing same since 144668 2019-12-10 15:37:57 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Igor Druzhinin Jan Beulich Jeremi Piotrowski Krzysztof Kolasa Mark Pryor Rasmus Villemoes Razvan
[Xen-devel] [ovmf test] 144676: regressions - FAIL
flight 144676 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/144676/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 144637 build-i386-xsm6 xen-buildfail REGR. vs. 144637 build-amd64-xsm 6 xen-buildfail REGR. vs. 144637 build-i3866 xen-buildfail REGR. vs. 144637 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf 7e55cf6b48dcd43de46d008b2f12caaad2554503 baseline version: ovmf 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 Last test of basis 144637 2019-12-09 09:09:49 Z1 days Failing since144646 2019-12-10 01:39:53 Z0 days4 attempts Testing same since 144661 2019-12-10 12:09:18 Z0 days2 attempts People who touched revisions under test: Bob Feng Jiewen Yao Steven Shi jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 Not pushing. commit 7e55cf6b48dcd43de46d008b2f12caaad2554503 Author: Jiewen Yao Date: Sat Dec 7 21:41:10 2019 +0800 SecurityPkg/Tcg2Smm: Measure the table before patch. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1940 According to TCG PFP specification: the ACPI table must be measured prior to any modification, and the measurement must be same cross every boot cycle. There is a fix 3a63c17ebc853cbb27d190729d01e27f68e65b94 for the HID data. However that is not enough. The LAML/LASA and PCD configuration change may also cause similar problem. We need measure the table before any update. Cc: Jian J Wang Cc: Chao Zhang Signed-off-by: Jiewen Yao Reviewed-by: Chao Zhang commit a80032dc44a1071a34f4415a7c5cef5170ee6159 Author: Steven Shi Date: Tue Nov 19 16:22:08 2019 +0800 BaseTools: Remove redundant binary cache file Redesign the binary cache and not need to save the cache intermediate result and state in memory as a ModuleBuildCacheIR class instance. So remove the CacheIR.py which define the ModuleBuildCacheIR class. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit fc8b8deac2d77524ff8cfe44acf95b5e1f59804e Author: Steven Shi Date: Tue Nov 19 16:17:00 2019 +0800 BaseTools: Leverage compiler output to optimize binary cache Redesign the binary cache and bases on the compiler to output the dependency header files info for every module. The binary cache will directly consume the dependency header files info and doesn't parse the C source code by iteself. Also redesign the dependency files list format for module and try to share the common lib hash result as more as possible in local process. Remove the unnecessary share data access across multiprocessing. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit 3bfbc915074a45f4d9c61aa2b698a62f1a24124e Author: Steven Shi Date: Mon Oct 21 14:51:49 2019 +0800 BaseTools: enhance the CacheCopyFile method arg names Enhance the CacheCopyFile method arg names to be
Re: [Xen-devel] [PATCH] x86/microcode: Support builtin CPU microcode
On 10.12.19 10:37, Jan Beulich wrote:
On 09.12.2019 09:41, Eslam Elnikety wrote:
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2113,7 +2113,7 @@ logic applies:
active by default.
### ucode (x86)
-> `= List of [ | scan=, nmi= ]`
+> `= List of [ | scan= | builtin=, nmi= ]`
Despite my other question regarding the usefulness of this as a
whole a few comments.
Do "scan" and "builtin" really need to exclude each other? I.e.
don't you mean , instead of | ?
The useful case here would be specifying ucode=scan,builtin which would
translate to fallback onto the builtin microcode if a scan fails. In
fact, this is already the case given the implementation in v1. It will
be better to clarify this semantic by allowing scan,builtin.
On that note, I really see the "" and "scan=" to be
equal. Following the logic earlier we should probably also allow
ucode=,builtin. This translates to fallback to builtin if there
are no modules at index .
@@ -2128,6 +2128,9 @@ when used with xen.efi (there the concept of modules
doesn't exist, and
the blob gets specified via the `ucode=` config file/section
entry; see [EFI configuration file description](efi.html)).
+'builtin' instructs the hypervisor to use the builtin microcode update. This
+option is available only if option BUILTIN_UCODE is enabled.
You also want to clarify its default - your reply to Andrew
suggested to me that only the negative form would really be
useful.
Indeed. This in any case will need a revamp to rework the ", instead of
|". Will address in v2.
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -218,6 +218,26 @@ config MEM_SHARING
bool "Xen memory sharing support" if EXPERT = "y"
depends on HVM
+config BUILTIN_UCODE
+ def_bool n
+ prompt "Support for Builtin Microcode"
These two lines should be folded into just
bool "Support for Builtin Microcode"
irrespective of other bad examples you may find in the code base.
The again ...
Will address in v2.
+ ---help---
+ Include the CPU microcode update in the Xen image itself. With this
+ support, Xen can update the CPU microcode upon boot using the builtin
+ microcode, with no need for an additional microcode boot modules.
+
+ If unsure, say N.
+
+config BUILTIN_UCODE_DIR
+ string
+ default "/lib/firmware"
+ depends on BUILTIN_UCODE
... are two separate options needed at all? Can't this latter one
being the empty string just imply the feature to be disabled?
I can go either way here. To me, two options is clearer.
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -7,6 +7,7 @@ subdir-y += mm
subdir-$(CONFIG_XENOPROF) += oprofile
subdir-$(CONFIG_PV) += pv
subdir-y += x86_64
+subdir-$(CONFIG_BUILTIN_UCODE) += microcode
Please respect the (half way?) alphabetical sorting here, unless
adding last is a requirement (in which case a brief comment should
say so, and why).
@@ -130,6 +138,10 @@ static int __init parse_ucode(const char *s)
{
if ( (val = parse_boolean("scan", s, ss)) >= 0 )
ucode_scan = val;
+#ifdef CONFIG_BUILTIN_UCODE
+ else if ( (val = parse_boolean("builtin", s, ss)) >= 0 )
Please watch out for hard tabs where they don't belong.
Good catch! Will fix in v2.
@@ -237,6 +249,48 @@ void __init microcode_grab_module(
scan:
if ( ucode_scan )
microcode_scan_module(module_map, mbi);
+
+#ifdef CONFIG_BUILTIN_UCODE
+/*
+ * Do not use the builtin microcode if:
+ * (a) builtin has been explicitly turned off (e.g., ucode=no-builtin)
+ * (b) a microcode module has been specified or a scan is successful
+ */
+if ( !ucode_builtin || ucode_mod.mod_end || ucode_blob.size )
+return;
+
+/* Set ucode_start/_end to the proper blob */
+if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+ucode_blob.size = (size_t)(__builtin_amd_ucode_end
+ - __builtin_amd_ucode_start);
+else if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
+ucode_blob.size = (size_t)(__builtin_intel_ucode_end
+ - __builtin_intel_ucode_start);
+else
+return;
+
+if ( !ucode_blob.size )
+{
+printk("No builtin ucode! 'ucode=builtin' is nullified.\n");
+return;
+}
+else if ( ucode_blob.size > MAX_EARLY_CPIO_MICROCODE )
With the "return" above please omit the "else" here. But why this
restriction, and ...
Will adjust in v2.
+{
+printk("Builtin microcode payload too big! (%ld, we can do %d)\n",
+ ucode_blob.size, MAX_EARLY_CPIO_MICROCODE);
+ucode_blob.size = 0;
+return;
+}
+
+ucode_blob.data = xmalloc_bytes(ucode_blob.size);
+if ( !ucode_blob.data )
+return;
+
+if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+
Re: [Xen-devel] [PATCH] x86/microcode: Support builtin CPU microcode
On 10.12.19 10:21, Jan Beulich wrote:
On 09.12.2019 22:49, Eslam Elnikety wrote:
On 09.12.19 16:19, Andrew Cooper wrote:
On 09/12/2019 08:41, Eslam Elnikety wrote:
--- /dev/null
+++ b/xen/arch/x86/microcode/Makefile
@@ -0,0 +1,40 @@
+# Copyright (C) 2019 Amazon.com, Inc. or its affiliates.
+# Author: Eslam Elnikety
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+obj-y += builtin_ucode.o
+
+# Directory holding the microcode updates.
+UCODE_DIR=$(patsubst "%",%,$(CONFIG_BUILTIN_UCODE_DIR))
+amd-blobs := $(wildcard $(UCODE_DIR)/amd-ucode/*)
+intel-blobs := $(wildcard $(UCODE_DIR)/intel-ucode/*)
This is a little dangerous. I can see why you want to do it like this,
and I can't provide any obvious suggestions, but if this glob picks up
anything which isn't a microcode file, it will break the logic to search
for the right blob.
We can limit the amd-blobs and intel-blob to binaries following the
naming convention AuthenticAMD.bin and GenuineIntel.bin for amd and
intel, respectively. Yet, this would be imposing an unnecessary
restriction on administrators who may want to be innovative with naming
(or want to use the naming microcode_amd_*.bin or FF-MM-SS instead).
Alternatively, we can introduce CONFIG_BUILTIN_UCODE_INTEL and
CONFIG_BUILTIN_UCODE_AMD. Both default to empty strings. Then, an
administrator can specify exactly the microcodes to include relative to
the CONFIG_BUILTIN_UCODE_DIR. For example:
CONFIG_BUILTIN_UCODE_INTEL="intel-ucode/06-3a-09"
CONFIG_BUILTIN_UCODE_AMD="amd-ucode/microcode_amd_fam15h.bin"
This would make the feature even less generic - I already meant to
I do not follow the point about being less generic. (I hope my example
did not give the false impression that CONFIG_BUILTIN_UCODE_{AMD,INTEL}
allow for only a single microcode blob for a single signature).
ask whether building ucode into binaries is really a useful thing
when we already have more flexible ways. I could see this being
useful if there was no other way to make ucode available at boot
time.
It is useful in addition to the existing ways to do early microcode
updates. First, when operating many hosts, using boot modules (either a
distinct microcode module or within an initrd) becomes involved. For
instance, tools to update boot entries (e.g.,
https://linux.die.net/man/8/grubby) do not support adding arbitrary
(microcode) modules.
Second, there is often need to couple a Xen build with a minimum
microcode patch level. Having the microcode built within the Xen image
itself is a streamlined, natural way of achieving that.
Thanks,
Eslam
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [seabios test] 144665: tolerable FAIL - PUSHED
flight 144665 seabios real [real] http://logs.test-lab.xenproject.org/osstest/logs/144665/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144198 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144198 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144198 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 144198 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass version targeted for testing: seabios f21b5a4aeb020f2a5e2c6503f906a9349dd2f069 baseline version: seabios c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d Last test of basis 144198 2019-11-18 14:08:47 Z 22 days Testing same since 144644 2019-12-09 21:08:58 Z1 days3 attempts People who touched revisions under test: Kevin O'Connor 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-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm pass test-amd64-i386-xl-qemuu-debianhvm-i386-xsm pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-qemuu-ws16-amd64 fail test-amd64-i386-xl-qemuu-ws16-amd64 fail test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrictpass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict pass test-amd64-amd64-qemuu-nested-intel pass test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow pass test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 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/seabios.git c9ba527..f21b5a4 f21b5a4aeb020f2a5e2c6503f906a9349dd2f069 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH AUTOSEL 4.19 120/177] xen/gntdev: Use select for DMA_SHARED_BUFFER
From: Jason Gunthorpe
[ Upstream commit fa6614d8ef13c63aac52ad7c07c5e69ce4aba3dd ]
DMA_SHARED_BUFFER can not be enabled by the user (it represents a library
set in the kernel). The kconfig convention is to use select for such
symbols so they are turned on implicitly when the user enables a kconfig
that needs them.
Otherwise the XEN_GNTDEV_DMABUF kconfig is overly difficult to enable.
Fixes: 932d6562179e ("xen/gntdev: Add initial support for dma-buf UAPI")
Cc: Oleksandr Andrushchenko
Cc: Boris Ostrovsky
Cc: xen-devel@lists.xenproject.org
Cc: Juergen Gross
Cc: Stefano Stabellini
Reviewed-by: Juergen Gross
Reviewed-by: Oleksandr Andrushchenko
Signed-off-by: Jason Gunthorpe
Signed-off-by: Juergen Gross
Signed-off-by: Sasha Levin
---
drivers/xen/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 90d387b50ab74..0505eeb593b5c 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -158,7 +158,8 @@ config XEN_GNTDEV
config XEN_GNTDEV_DMABUF
bool "Add support for dma-buf grant access device driver extension"
- depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC && DMA_SHARED_BUFFER
+ depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC
+ select DMA_SHARED_BUFFER
help
Allows userspace processes and kernel modules to use Xen backed
dma-buf implementation. With this extension grant references to
--
2.20.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH AUTOSEL 5.4 234/350] xen/gntdev: Use select for DMA_SHARED_BUFFER
From: Jason Gunthorpe
[ Upstream commit fa6614d8ef13c63aac52ad7c07c5e69ce4aba3dd ]
DMA_SHARED_BUFFER can not be enabled by the user (it represents a library
set in the kernel). The kconfig convention is to use select for such
symbols so they are turned on implicitly when the user enables a kconfig
that needs them.
Otherwise the XEN_GNTDEV_DMABUF kconfig is overly difficult to enable.
Fixes: 932d6562179e ("xen/gntdev: Add initial support for dma-buf UAPI")
Cc: Oleksandr Andrushchenko
Cc: Boris Ostrovsky
Cc: xen-devel@lists.xenproject.org
Cc: Juergen Gross
Cc: Stefano Stabellini
Reviewed-by: Juergen Gross
Reviewed-by: Oleksandr Andrushchenko
Signed-off-by: Jason Gunthorpe
Signed-off-by: Juergen Gross
Signed-off-by: Sasha Levin
---
drivers/xen/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 79cc75096f423..a50dadd010933 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -141,7 +141,8 @@ config XEN_GNTDEV
config XEN_GNTDEV_DMABUF
bool "Add support for dma-buf grant access device driver extension"
- depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC && DMA_SHARED_BUFFER
+ depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC
+ select DMA_SHARED_BUFFER
help
Allows userspace processes and kernel modules to use Xen backed
dma-buf implementation. With this extension grant references to
--
2.20.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 144672: tolerable all pass - PUSHED
flight 144672 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/144672/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 272c18435e93cbf749c096a9552ab5ef0d79a4ca baseline version: xen 4935a5433db28077fe6313f920bbedcd54516cec Last test of basis 144659 2019-12-10 11:00:42 Z0 days Testing same since 144672 2019-12-10 18:00:40 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Daniel De Graaf George Dunlap Julien Grall 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 4935a5433d..272c18435e 272c18435e93cbf749c096a9552ab5ef0d79a4ca -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 144661: regressions - FAIL
flight 144661 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/144661/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 144637 build-i386-xsm6 xen-buildfail REGR. vs. 144637 build-amd64-xsm 6 xen-buildfail REGR. vs. 144637 build-i3866 xen-buildfail REGR. vs. 144637 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a version targeted for testing: ovmf 7e55cf6b48dcd43de46d008b2f12caaad2554503 baseline version: ovmf 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 Last test of basis 144637 2019-12-09 09:09:49 Z1 days Failing since144646 2019-12-10 01:39:53 Z0 days3 attempts Testing same since 144661 2019-12-10 12:09:18 Z0 days1 attempts People who touched revisions under test: Bob Feng Jiewen Yao Steven Shi jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 Not pushing. commit 7e55cf6b48dcd43de46d008b2f12caaad2554503 Author: Jiewen Yao Date: Sat Dec 7 21:41:10 2019 +0800 SecurityPkg/Tcg2Smm: Measure the table before patch. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1940 According to TCG PFP specification: the ACPI table must be measured prior to any modification, and the measurement must be same cross every boot cycle. There is a fix 3a63c17ebc853cbb27d190729d01e27f68e65b94 for the HID data. However that is not enough. The LAML/LASA and PCD configuration change may also cause similar problem. We need measure the table before any update. Cc: Jian J Wang Cc: Chao Zhang Signed-off-by: Jiewen Yao Reviewed-by: Chao Zhang commit a80032dc44a1071a34f4415a7c5cef5170ee6159 Author: Steven Shi Date: Tue Nov 19 16:22:08 2019 +0800 BaseTools: Remove redundant binary cache file Redesign the binary cache and not need to save the cache intermediate result and state in memory as a ModuleBuildCacheIR class instance. So remove the CacheIR.py which define the ModuleBuildCacheIR class. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit fc8b8deac2d77524ff8cfe44acf95b5e1f59804e Author: Steven Shi Date: Tue Nov 19 16:17:00 2019 +0800 BaseTools: Leverage compiler output to optimize binary cache Redesign the binary cache and bases on the compiler to output the dependency header files info for every module. The binary cache will directly consume the dependency header files info and doesn't parse the C source code by iteself. Also redesign the dependency files list format for module and try to share the common lib hash result as more as possible in local process. Remove the unnecessary share data access across multiprocessing. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit 3bfbc915074a45f4d9c61aa2b698a62f1a24124e Author: Steven Shi Date: Mon Oct 21 14:51:49 2019 +0800 BaseTools: enhance the CacheCopyFile method arg names Enhance the CacheCopyFile method arg names to be
[Xen-devel] [xen-4.13-testing test] 144655: trouble: broken/fail/pass
flight 144655 xen-4.13-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/144655/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-xtf-amd64-amd64-4 broken test-xtf-amd64-amd64-2 broken test-amd64-amd64-xl-qemut-debianhvm-amd64 broken test-amd64-amd64-xl-xsm broken test-amd64-i386-xl-qemut-debianhvm-i386-xsm broken test-amd64-amd64-pygrub broken test-amd64-amd64-pygrub 4 host-install(4)broken REGR. vs. 144609 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm broken in 144640 test-xtf-amd64-amd64-1 broken in 144640 test-amd64-amd64-i386-pvgrub broken in 144640 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow broken in 144640 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow broken in 144640 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm broken in 144645 test-amd64-amd64-xl-shadow broken in 144645 test-amd64-i386-xl-xsm broken in 144645 test-amd64-i386-xl-qemuu-debianhvm-amd64 broken in 144645 test-amd64-amd64-xl-credit1 broken in 144645 Tests which are failing intermittently (not blocking): test-amd64-amd64-i386-pvgrub 4 host-install(4) broken in 144640 pass in 144655 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 4 host-install(4) broken in 144640 pass in 144655 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 4 host-install(4) broken in 144640 pass in 144655 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 4 host-install(4) broken in 144640 pass in 144655 test-xtf-amd64-amd64-1 4 host-install(4) broken in 144640 pass in 144655 test-amd64-amd64-xl-shadow 4 host-install(4) broken in 144645 pass in 144655 test-amd64-amd64-xl-credit1 4 host-install(4) broken in 144645 pass in 144655 test-amd64-i386-xl-qemuu-debianhvm-amd64 4 host-install(4) broken in 144645 pass in 144655 test-amd64-i386-xl-xsm 4 host-install(4) broken in 144645 pass in 144655 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 4 host-install(4) broken in 144645 pass in 144655 test-amd64-amd64-xl-qemut-debianhvm-amd64 4 host-install(4) broken pass in 144640 test-xtf-amd64-amd64-44 host-install(4) broken pass in 144645 test-amd64-amd64-xl-xsm 4 host-install(4) broken pass in 144645 test-xtf-amd64-amd64-24 host-install(4) broken pass in 144645 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 4 host-install(4) broken pass in 144645 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail in 144640 pass in 144655 test-amd64-amd64-libvirt-vhd 10 debian-di-install fail in 144640 pass in 144655 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 12 guest-start/debianhvm.repeat fail in 144645 pass in 144655 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail pass in 144640 test-amd64-amd64-xl-shadow 18 guest-localmigrate/x10 fail pass in 144640 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail pass in 144645 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 17 guest-start.2 fail in 144645 REGR. vs. 144609 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass
[Xen-devel] [tip: x86/cleanups] x86/Kconfig: Fix Kconfig indentation
The following commit has been merged into the x86/cleanups branch of tip: Commit-ID: b03b016fe54edd1b527d749e139b2fc9407ac414 Gitweb: https://git.kernel.org/tip/b03b016fe54edd1b527d749e139b2fc9407ac414 Author:Krzysztof Kozlowski AuthorDate:Thu, 21 Nov 2019 04:21:09 +01:00 Committer: Borislav Petkov CommitterDate: Tue, 10 Dec 2019 18:43:21 +01:00 x86/Kconfig: Fix Kconfig indentation Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski Signed-off-by: Borislav Petkov Cc: "H. Peter Anvin" Cc: Boris Ostrovsky Cc: Ingo Molnar Cc: Juergen Gross Cc: Stefano Stabellini Cc: Thomas Gleixner Cc: x86-ml Cc: xen-devel@lists.xenproject.org Link: https://lkml.kernel.org/r/1574306470-10305-1-git-send-email-k...@kernel.org --- arch/x86/Kconfig | 68 +-- arch/x86/xen/Kconfig | 8 ++--- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 5e89499..d7bbed5 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -439,8 +439,8 @@ config X86_MPPARSE (esp with 64bit cpus) with acpi support, MADT and DSDT will override it config GOLDFISH - def_bool y - depends on X86_GOLDFISH + def_bool y + depends on X86_GOLDFISH config RETPOLINE bool "Avoid speculative indirect branches in kernel" @@ -561,9 +561,9 @@ config X86_UV # Please maintain the alphabetic order if and when there are additions config X86_GOLDFISH - bool "Goldfish (Virtual Platform)" - depends on X86_EXTENDED_PLATFORM - ---help--- + bool "Goldfish (Virtual Platform)" + depends on X86_EXTENDED_PLATFORM + ---help--- Enable support for the Goldfish virtual platform used primarily for Android development. Unless you are building for the Android Goldfish emulator say N here. @@ -806,9 +806,9 @@ config KVM_GUEST timing infrastructure such as time of day, and system time config ARCH_CPUIDLE_HALTPOLL -def_bool n -prompt "Disable host haltpoll when loading haltpoll driver" -help + def_bool n + prompt "Disable host haltpoll when loading haltpoll driver" + help If virtualized under KVM, disable host haltpoll. config PVH @@ -887,16 +887,16 @@ config HPET_EMULATE_RTC depends on HPET_TIMER && (RTC=y || RTC=m || RTC_DRV_CMOS=m || RTC_DRV_CMOS=y) config APB_TIMER - def_bool y if X86_INTEL_MID - prompt "Intel MID APB Timer Support" if X86_INTEL_MID - select DW_APB_TIMER - depends on X86_INTEL_MID && SFI - help - APB timer is the replacement for 8254, HPET on X86 MID platforms. - The APBT provides a stable time base on SMP - systems, unlike the TSC, but it is more expensive to access, - as it is off-chip. APB timers are always running regardless of CPU - C states, they are used as per CPU clockevent device when possible. + def_bool y if X86_INTEL_MID + prompt "Intel MID APB Timer Support" if X86_INTEL_MID + select DW_APB_TIMER + depends on X86_INTEL_MID && SFI + help +APB timer is the replacement for 8254, HPET on X86 MID platforms. +The APBT provides a stable time base on SMP +systems, unlike the TSC, but it is more expensive to access, +as it is off-chip. APB timers are always running regardless of CPU +C states, they are used as per CPU clockevent device when possible. # Mark as expert because too many people got it wrong. # The code disables itself when not needed. @@ -1035,8 +1035,8 @@ config SCHED_MC_PRIO If unsure say Y here. config UP_LATE_INIT - def_bool y - depends on !SMP && X86_LOCAL_APIC + def_bool y + depends on !SMP && X86_LOCAL_APIC config X86_UP_APIC bool "Local APIC support on uniprocessors" if !PCI_MSI @@ -1185,8 +1185,8 @@ config X86_LEGACY_VM86 If unsure, say N here. config VM86 - bool - default X86_LEGACY_VM86 + bool + default X86_LEGACY_VM86 config X86_16BIT bool "Enable support for 16-bit segments" if EXPERT @@ -1207,10 +1207,10 @@ config X86_ESPFIX64 depends on X86_16BIT && X86_64 config X86_VSYSCALL_EMULATION - bool "Enable vsyscall emulation" if EXPERT - default y - depends on X86_64 - ---help--- + bool "Enable vsyscall emulation" if EXPERT + default y + depends on X86_64 + ---help--- This enables emulation of the legacy vsyscall page. Disabling it is roughly equivalent to booting with vsyscall=none, except that it will also disable the helpful warning if a program @@ -1648,9 +1648,9 @@ config ARCH_PROC_KCORE_TEXT depends on X86_64 && PROC_KCORE config ILLEGAL_POINTER_VALUE -
Re: [Xen-devel] [PATCH 5/6] xen/tasklet: Return -ERESTART from continue_hypercall_on_cpu()
On 10/12/2019 08:55, Jan Beulich wrote: > On 09.12.2019 18:49, Andrew Cooper wrote: >> On 09/12/2019 16:52, Jan Beulich wrote: >>> On 05.12.2019 23:30, Andrew Cooper wrote: Some hypercalls tasklets want to create a continuation, rather than fail the hypercall with a hard error. By the time the tasklet is executing, it is too late to create the continuation, and even continue_hypercall_on_cpu() doesn't have enough state to do it correctly. >>> I think it would be quite nice if you made clear what piece of state >>> it is actually missing. To be honest, I don't recall anymore. >> How to correctly mutate the registers and/or memory (which is specific >> to the hypercall subop in some cases). > Well, in-memory arguments can be accessed as long as the mapping is > the right one (which it typically wouldn't be inside a tasklet). Do > existing continue_hypercall_on_cpu() users need this? Looking over > patch 4 again, I didn't think so. (Which isn't to say that removing > the latent issue is not a good thing.) > > In-register values can be changed as long as the respective exit > path will suitably pick up the value, which I thought was always > the case. > > Hence I'm afraid your single reply sentence didn't really clarify > matters. I'm sorry if this is just because of me being dense. How, physically, would you arrange for continue_hypercall_on_cpu() to make the requisite state adjustments? Yes - registers and memory can be accessed, but only the hypercall (sub?)op handler knows how to mutate them appropriately. You'd have to copy the mutation logic into continue_hypercall_on_cpu(), and pass in op/subops and a union of all pointers, *and* whatever intermediate state the subop handler needs. Or you can return -ERESTART and let the caller DTRT with the state it has in context, as it would in other cases requiring a continuation. > There is one RFC point. The statement in the header file of "If this function returns 0 then the function is guaranteed to run at some point in the future." was never true. In the case of a CPU miss, the hypercall would be blindly failed with -EINVAL. >>> "Was never true" sounds like "completely broken". Afaict it was true >>> in all cases except the purely hypothetical one of the tasklet ending >>> up executing on the wrong CPU. >> There is nothing hypothetical about it. It really will go wrong when a >> CPU gets offlined. > Accepted, but it's still not like "completely broken". I didn't mean it like that. I mean "it has never had the property it claimed", which is distinct from "the claim used to be true, but was then accidentally regressed". > I would even > suppose the case wasn't considered when CPU offlining support was > introduced, not when continue_hypercall_on_cpu() came into existence > (which presumably is when the comment was written). > > Anyway - yes, I agree this is a fair solution to the issue at hand. > The current behaviour with this patch is to not cancel the continuation, which I think is less bad, but still not great. Thoughts? >>> Well, that's a guest live lock then aiui. >> It simply continues again. It will livelock only if the hypercall picks >> a bad cpu all the time. > Oh, I see I was mislead by continue_hypercall_tasklet_handler() not > updating info->cpu, not paying attention to it actually freeing info. > Plus a crucial aspect looks to be that there are no "chained" uses of > continue_hypercall_on_cpu() anymore (the microcode loading one being > gone now) - afaict any such wouldn't guarantee forward progress with > this new model (without recording somewhere which CPUs had been dealt > with already). I'd forgotten that we had that, but I can't say I'm sad to see the back of it. I recall at the time saying that it wasn't a clever move. For now, I suggest that we ignore that case. If an when a real usecase appears, we can consider making adjustments. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 144650: trouble: broken/fail/pass
flight 144650 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/144650/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt broken test-amd64-i386-xl-shadowbroken test-amd64-i386-xl-qemuu-debianhvm-i386-xsm broken test-amd64-i386-libvirt-xsm broken test-amd64-i386-qemuu-rhel6hvm-intel broken test-amd64-amd64-pygrub broken test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm broken test-amd64-amd64-pygrub 4 host-install(4)broken REGR. vs. 144591 test-amd64-i386-xl-shadow 4 host-install(4)broken REGR. vs. 144591 test-amd64-i386-libvirt 4 host-install(4)broken REGR. vs. 144591 test-amd64-amd64-amd64-pvgrub broken in 144643 Tests which are failing intermittently (not blocking): test-amd64-amd64-amd64-pvgrub 4 host-install(4) broken in 144643 pass in 144650 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 4 host-install(4) broken pass in 144643 test-amd64-i386-qemuu-rhel6hvm-intel 4 host-install(4) broken pass in 144643 test-amd64-i386-libvirt-xsm 4 host-install(4) broken pass in 144643 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 4 host-install(4) broken pass in 144643 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail pass in 144643 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail pass in 144643 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 16 guest-localmigrate fail blocked in 144591 test-amd64-amd64-xl-rtds 17 guest-saverestore.2 fail in 144643 blocked in 144591 test-amd64-i386-libvirt-xsm 13 migrate-support-check fail in 144643 never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail in 144643 never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stopfail in 144643 never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144591 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 144591 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144591 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 144591 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144591 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13
[Xen-devel] baroque0 (was Re: [xen-4.13-testing test] 144645: trouble: broken/fail/pass)
Jürgen Groß writes ("Re: [Xen-devel] [xen-4.13-testing test] 144645: trouble:
broken/fail/pass"):
> On 10.12.19 10:06, osstest service owner wrote:
> > flight 144645 xen-4.13-testing real [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/144645/
>
> Something wrong with baroque0?
>
> Ian, can you please check ASAP?
I have unblessed it. It seems dead.
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 7/7] x86: implement Hyper-V clock source
On 25.10.2019 11:16, Wei Liu wrote:
> @@ -614,6 +615,89 @@ static struct platform_timesource __initdata
> plt_xen_timer =
> };
> #endif
>
> +#ifdef CONFIG_HYPERV_GUEST
> +/
> + * PLATFORM TIMER 6: HYPER-V REFERENCE TSC
I don't think numbering is very helpful for optionally built code.
(I realize though that this same anomaly exists for the Xen guest
timer already.)
> + */
> +
> +static struct ms_hyperv_tsc_page hyperv_tsc_page __aligned(PAGE_SIZE);
Does this need to be a statically allocated page?
> +static int64_t __init init_hyperv_timer(struct platform_timesource *pts)
> +{
> +unsigned long maddr;
paddr_t ?
> +uint64_t tsc_msr, freq;
> +
> +if ( !hyperv_guest ||
> + !(ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE) )
Is the hyperv_guest check really needed? The feature bit won't be
set without that variable being true anyway, will it?
> +return 0;
> +
> +maddr = virt_to_maddr(_tsc_page);
> +
> +/*
> + * Per Hyper-V TLFS:
> + * 1. Read existing MSR value
> + * 2. Preserve bits [11:1]
> + * 3. Set bits [63:12] to be guest physical address of tsc page
> + * 4. Set enabled bit (0)
> + * 5. Write back new MSR value
> + */
> +rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr);
> +tsc_msr &= GENMASK_ULL(11, 1);
A discussion not so long ago has resulted in, iirc, Andrew and me
agreeing that in its current shape we don't want to see any uses
of this macro outside of Arm-specific code.
> +tsc_msr = tsc_msr | (uint64_t)maddr | 1 /* enabled */;
Why the cast? And maybe easier as "tsc_msr |= "?
> +wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr);
> +
> +/* Get TSC frequency from Hyper-V */
> +rdmsrl(HV_X64_MSR_TSC_FREQUENCY, freq);
> +pts->frequency = freq;
> +
> +return freq;
> +}
> +
> +static inline uint64_t read_hyperv_timer(void)
> +{
> +uint64_t scale, offset, ret, tsc;
> +uint32_t seq;
> +struct ms_hyperv_tsc_page *tsc_page = _tsc_page;
const?
> +do {
> +seq = tsc_page->tsc_sequence;
> +
> +/* Seq 0 is special. It means the TSC enlightenment is not
> + * available at the moment. The reference time can only be
> + * obtained from the Reference Counter MSR.
> + */
> +if ( seq == 0 )
> +{
> +rdmsrl(HV_X64_MSR_TIME_REF_COUNT, ret);
> +return ret;
> +}
> +
> +smp_rmb();
> +
> +tsc = rdtsc_ordered();
This already includes at least a read fence.
> +scale = tsc_page->tsc_scale;
> +offset = tsc_page->tsc_offset;
> +
> +smp_rmb();
> +
> +} while (tsc_page->tsc_sequence != seq);
> +
> +/* x86 has ARCH_SUPPORTS_INT128 */
> +ret = (uint64_t)(((__uint128_t)tsc * scale) >> 64) + offset;
The final cast isn't really needed, is it? As to the multiplication
- are you sure all compilers in all cases will avoid falling back
to a library call here? In other similar places I think we use
inline assembly instead.
> +return ret;
> +}
> +
> +static struct platform_timesource __initdata plt_hyperv_timer =
> +{
> +.id = "hyperv",
> +.name = "HYPER-V REFERENCE TSC",
> +.read_counter = read_hyperv_timer,
> +.init = init_hyperv_timer,
> +.counter_bits = 63,
Why 63? The calculation above is a uint64_t one. If there are
wrapping concerns like for the TSC source, please add a
respective comment (which may be as brief as a reference to
the other one, if that's appropriate).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 6/7] x86/hyperv: provide hyperv_guest variable
On 25.10.2019 11:16, Wei Liu wrote: > --- a/xen/arch/x86/guest/hyperv/hyperv.c > +++ b/xen/arch/x86/guest/hyperv/hyperv.c > @@ -24,6 +24,7 @@ > #include > > struct ms_hyperv_info ms_hyperv; > +bool hyperv_guest; With __read_mostly added here (perhaps also in the earlier patch for the adjacent one) Acked-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf bisection] complete build-amd64
branch xen-unstable xenbranch xen-unstable job build-amd64 testid xen-build Tree: ovmf https://github.com/tianocore/edk2.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: ovmf https://github.com/tianocore/edk2.git Bug introduced: 13c5e34a1b8bfedbd10ea038cfcbae5caeab6652 Bug not present: 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/144667/ commit 13c5e34a1b8bfedbd10ea038cfcbae5caeab6652 Author: Bob Feng Date: Mon Dec 2 16:24:19 2019 +0800 BaseTools: Add build option for dependency file generation BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2311 Add /showIncludes for msvc and -MMD -MF $@.deps for GCC and CLANG Remove /MP for msvc since /MP does not work with /showIncludes Signed-off-by: Bob Feng Cc: Liming Gao Cc: Steven Shi Cc: Michael D Kinney Reviewed-by: Liming Gao For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/ovmf/build-amd64.xen-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/ovmf/build-amd64.xen-build --summary-out=tmp/144667.bisection-summary --basis-template=144637 --blessings=real,real-bisect ovmf build-amd64 xen-build Searching for failure / basis pass: 144651 fail [host=godello1] / 144637 [host=italia0] 144590 ok. Failure / basis pass flights: 144651 / 144590 (tree with no url: minios) Tree: ovmf https://github.com/tianocore/edk2.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest a80032dc44a1071a34f4415a7c5cef5170ee6159 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 Basis pass 49054b6bb66d35484e92c65f27584c4283a60986 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d 131c89ce1e1dfd0b57a249615a92de4f120d9100 Generating revisions with ./adhoc-revtuple-generator https://github.com/tianocore/edk2.git#49054b6bb66d35484e92c65f27584c4283a60986-a80032dc44a1071a34f4415a7c5cef5170ee6159 git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git#933ebad2470a169504799a1d95b8e410bd9847ef-933ebad2470a169504799a1d95b8e410bd9847ef git://xenbits.xen.org/osstest/seabios.git#c9ba5276e3217ac6a1ec772dbebf568ba3a8a5\ 5d-c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d git://xenbits.xen.org/xen.git#131c89ce1e1dfd0b57a249615a92de4f120d9100-ae25407faaaddf4abe44137ebf0e177a8c8f9858 Loaded 10001 nodes in revision graph Searching for test results: 144590 pass 49054b6bb66d35484e92c65f27584c4283a60986 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d 131c89ce1e1dfd0b57a249615a92de4f120d9100 144637 [host=italia0] 144646 fail 0c3e8e9947a6c13b4327dd11b20acb95441701cf d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144652 pass 49054b6bb66d35484e92c65f27584c4283a60986 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d 131c89ce1e1dfd0b57a249615a92de4f120d9100 144653 fail 0c3e8e9947a6c13b4327dd11b20acb95441701cf d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144654 pass 49054b6bb66d35484e92c65f27584c4283a60986 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144656 fail 13c5e34a1b8bfedbd10ea038cfcbae5caeab6652 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144651 fail a80032dc44a1071a34f4415a7c5cef5170ee6159 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d ae25407faaaddf4abe44137ebf0e177a8c8f9858 144658 pass 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef
Re: [Xen-devel] [PATCH for-next 5/7] x86: use running_on_hypervisor to gate hypervisor_setup
On 25.10.2019 11:16, Wei Liu wrote: > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1577,7 +1577,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) > max_cpus = nr_cpu_ids; > } > > -if ( xen_guest ) > +if ( running_on_hypervisor ) > hypervisor_setup(); This code is using hypervisor_name already, so I guess the patch has become unnecessary? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 4/7] x86: add a comment regarding the location of hypervisor_probe
On 25.10.2019 11:16, Wei Liu wrote: > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 3/7] x86/hyperv: extract more information from Hyper-V
On 25.10.2019 11:16, Wei Liu wrote:
> --- a/xen/arch/x86/guest/hyperv/hyperv.c
> +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> @@ -21,6 +21,9 @@
> #include
>
> #include
> +#include
> +
> +struct ms_hyperv_info ms_hyperv;
>
> bool __init hyperv_probe(void)
> {
> @@ -36,6 +39,17 @@ bool __init hyperv_probe(void)
> if ( eax != 0x31237648 )/* Hv#1 */
> return false;
>
> +/* Extract more information from Hyper-V */
> +ms_hyperv.features = cpuid_eax(HYPERV_CPUID_FEATURES);
> +ms_hyperv.misc_features = cpuid_edx(HYPERV_CPUID_FEATURES);
It's __init code, so it doesn't matter all that much, but perhaps
obtain these two and ...
> +ms_hyperv.hints = cpuid_eax(HYPERV_CPUID_ENLIGHTMENT_INFO);
> +
> +if ( ms_hyperv.hints & HV_X64_ENLIGHTENED_VMCS_RECOMMENDED )
> +ms_hyperv.nested_features = cpuid_eax(HYPERV_CPUID_NESTED_FEATURES);
> +
> +ms_hyperv.max_vp_index = cpuid_eax(HYPERV_CPUID_IMPLEMENT_LIMITS);
> +ms_hyperv.max_lp_index = cpuid_ebx(HYPERV_CPUID_IMPLEMENT_LIMITS);
... these two with just one CPUID invocation each? Preferably
with this adjustment
Acked-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 19/22] golang/xenlight: implement keyed union Go to C marshaling
From: Nick Rosbrook
Since the C union cannot be directly populated, populate the fields of the
corresponding C struct defined in the cgo preamble, and then copy that
struct as bytes into the byte slice that Go uses as the union.
Signed-off-by: Nick Rosbrook
---
tools/golang/xenlight/gengotypes.py | 77 ++-
tools/golang/xenlight/helpers.gen.go | 325 +++
2 files changed, 400 insertions(+), 2 deletions(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index cb9b18218f..1839ac14b6 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -504,8 +504,7 @@ def xenlight_golang_define_to_C(ty = None, typename = None,
nested = False):
s += xenlight_golang_define_to_C(f.type, typename=f.name,
nested=True)
elif isinstance(f.type, idl.KeyedUnion):
-# TODO
-pass
+s += xenlight_golang_union_to_C(f.type, f.name, ty.typename,
ty.dispose_fn)
else:
raise Exception('type {} not supported'.format(f.type))
@@ -516,6 +515,80 @@ def xenlight_golang_define_to_C(ty = None, typename =
None, nested = False):
return s
+def xenlight_golang_union_to_C(ty = None, union_name = '',
+ struct_name = '', dispose_fn = ''):
+keyname = ty.keyvar.name
+gokeyname = xenlight_golang_fmt_name(keyname)
+keytype = ty.keyvar.type.typename
+gokeytype = xenlight_golang_fmt_name(keytype)
+
+interface_name = '{}_{}_union'.format(struct_name, keyname)
+interface_name = xenlight_golang_fmt_name(interface_name, exported=False)
+
+cgo_keyname = keyname
+if cgo_keyname in go_keywords:
+cgo_keyname = '_' + cgo_keyname
+
+
+s = 'xc.{} = C.{}(x.{})\n'.format(cgo_keyname,keytype,gokeyname)
+s += 'switch x.{}{{\n'.format(gokeyname)
+
+# Create switch statement to determine how to populate the C union.
+for f in ty.fields:
+key_val = '{}_{}'.format(keytype, f.name)
+key_val = xenlight_golang_fmt_name(key_val)
+if f.type is None:
+continue
+
+s += 'case {}:\n'.format(key_val)
+cgotype = '{}_{}_union_{}'.format(struct_name,keyname,f.name)
+gotype = xenlight_golang_fmt_name(cgotype)
+goname = '{}_{}'.format(keyname,f.name)
+goname = xenlight_golang_fmt_name(goname,exported=False)
+
+field_name = xenlight_golang_fmt_name('{}_union'.format(keyname))
+s += 'tmp, ok := x.{}.({})\n'.format(field_name,gotype)
+s += 'if !ok {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,errors.New("wrong type for union key
{}")\n'.format(keyname)
+s += '}\n'
+
+s += 'var {} C.{}\n'.format(f.name,cgotype)
+for uf in f.type.fields:
+gotypename = xenlight_golang_fmt_name(uf.type.typename)
+ctypename = uf.type.typename
+gofname= xenlight_golang_fmt_name(uf.name)
+
+is_castable = (uf.type.json_parse_type == 'JSON_INTEGER' or
+ isinstance(uf.type, idl.Enumeration) or
+ gotypename in go_builtin_types)
+
+if not is_castable:
+s += '{}.{}, err =
tmp.{}.toC()\n'.format(f.name,uf.name,gofname)
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,err \n}\n'
+
+elif gotypename == 'string':
+s += '{}.{} =
C.CString(tmp.{})\n'.format(f.name,uf.name,gofname)
+
+else:
+s += '{}.{} =
C.{}(tmp.{})\n'.format(f.name,uf.name,ctypename,gofname)
+
+# The union is still represented as Go []byte.
+s += '{}Bytes :=
C.GoBytes(unsafe.Pointer(&{}),C.sizeof_{})\n'.format(f.name,
+
f.name,
+
cgotype)
+s += 'copy(xc.{}[:],{}Bytes)\n'.format(union_name,f.name)
+
+# End switch statement
+s += 'default:\n'
+err_string = '"invalid union key \'%v\'", x.{}'.format(gokeyname)
+s += 'return xc, fmt.Errorf({})'.format(err_string)
+s += '}\n'
+
+return s
+
def xenlight_golang_fmt_name(name, exported = True):
"""
Take a given type name and return an
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index a155b091a7..73ad5e9761 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -326,6 +326,21 @@ func (x *Channelinfo) toC() (xc C.libxl_channelinfo, err
error) {
xc.state = C.int(x.State)
xc.evtch = C.int(x.Evtch)
xc.rref = C.int(x.Rref)
+ xc.connection = C.libxl_channel_connection(x.Connection)
+ switch x.Connection {
+ case ChannelConnectionPty:
+ tmp, ok :=
[Xen-devel] [PATCH v3 21/22] golang/xenlight: revise use of Context type
From: Nick Rosbrook
Remove the exported global context variable, 'Ctx.' Generally, it is
better to not export global variables for use through a Go package.
However, there are some exceptions that can be found in the standard
library.
Add a NewContext function instead, and remove the Open, IsOpen, and
CheckOpen functions as a result.
Also, comment-out an ineffectual assignment to 'err' inside the function
Context.CpupoolInfo so that compilation does not fail.
Signed-off-by: Nick Rosbrook
---
tools/golang/xenlight/xenlight.go | 219 +-
1 file changed, 34 insertions(+), 185 deletions(-)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index f32eb11384..1c431fa4e5 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -74,6 +74,39 @@ func (e Error) Error() string {
return fmt.Sprintf("libxl error: %d", -e)
}
+// Context represents a libxl_ctx.
+type Context struct {
+ ctx*C.libxl_ctx
+ logger *C.xentoollog_logger_stdiostream
+}
+
+// NewContext returns a new Context.
+func NewContext() (*Context, error) {
+ var ctx Context
+
+ ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
+
+ ret := C.libxl_ctx_alloc(, C.LIBXL_VERSION, 0,
(*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
+ if ret != 0 {
+ return nil, Error(ret)
+ }
+
+ return , nil
+}
+
+// Close closes the Context.
+func (ctx *Context) Close() error {
+ ret := C.libxl_ctx_free(ctx.ctx)
+ ctx.ctx = nil
+ C.xtl_logger_destroy((*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
+
+ if ret != 0 {
+ return Error(ret)
+ }
+
+ return nil
+}
+
/*
* Types: Builtins
*/
@@ -298,11 +331,6 @@ func (cpl CpuidPolicyList) toC()
(C.libxl_cpuid_policy_list, error) {
return ccpl, nil
}
-type Context struct {
- ctx*C.libxl_ctx
- logger *C.xentoollog_logger_stdiostream
-}
-
// Hwcap represents a libxl_hwcap.
type Hwcap [8]uint32
@@ -453,11 +481,6 @@ func SchedulerFromString(name string) (s Scheduler, err
error) {
// libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool_out);
// void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nb_pool);
func (Ctx *Context) ListCpupool() (list []Cpupoolinfo) {
- err := Ctx.CheckOpen()
- if err != nil {
- return
- }
-
var nbPool C.int
c_cpupool_list := C.libxl_list_cpupool(Ctx.ctx, )
@@ -481,16 +504,11 @@ func (Ctx *Context) ListCpupool() (list []Cpupoolinfo) {
// int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t
poolid);
func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo) {
- err := Ctx.CheckOpen()
- if err != nil {
- return
- }
-
var c_cpupool C.libxl_cpupoolinfo
ret := C.libxl_cpupool_info(Ctx.ctx, _cpupool, C.uint32_t(Poolid))
if ret != 0 {
- err = Error(-ret)
+ //err = Error(-ret)
return
}
defer C.libxl_cpupoolinfo_dispose(_cpupool)
@@ -507,11 +525,6 @@ func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool
Cpupoolinfo) {
// FIXME: uuid
// FIXME: Setting poolid
func (Ctx *Context) CpupoolCreate(Name string, Scheduler Scheduler, Cpumap
Bitmap) (err error, Poolid uint32) {
- err = Ctx.CheckOpen()
- if err != nil {
- return
- }
-
poolid := C.uint32_t(C.LIBXL_CPUPOOL_POOLID_ANY)
name := C.CString(Name)
defer C.free(unsafe.Pointer(name))
@@ -540,11 +553,6 @@ func (Ctx *Context) CpupoolCreate(Name string, Scheduler
Scheduler, Cpumap Bitma
// int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);
func (Ctx *Context) CpupoolDestroy(Poolid uint32) (err error) {
- err = Ctx.CheckOpen()
- if err != nil {
- return
- }
-
ret := C.libxl_cpupool_destroy(Ctx.ctx, C.uint32_t(Poolid))
if ret != 0 {
err = Error(-ret)
@@ -556,11 +564,6 @@ func (Ctx *Context) CpupoolDestroy(Poolid uint32) (err
error) {
// int libxl_cpupool_cpuadd(libxl_ctx *ctx, uint32_t poolid, int cpu);
func (Ctx *Context) CpupoolCpuadd(Poolid uint32, Cpu int) (err error) {
- err = Ctx.CheckOpen()
- if err != nil {
- return
- }
-
ret := C.libxl_cpupool_cpuadd(Ctx.ctx, C.uint32_t(Poolid), C.int(Cpu))
if ret != 0 {
err = Error(-ret)
@@ -573,11 +576,6 @@ func (Ctx *Context) CpupoolCpuadd(Poolid uint32, Cpu int)
(err error) {
// int libxl_cpupool_cpuadd_cpumap(libxl_ctx *ctx, uint32_t poolid,
// const libxl_bitmap *cpumap);
func (Ctx *Context) CpupoolCpuaddCpumap(Poolid uint32, Cpumap Bitmap) (err
error) {
- err = Ctx.CheckOpen()
- if err != nil {
- return
- }
-
cbm, err := Cpumap.toC()
if err
[Xen-devel] [PATCH v3 18/22] golang/xenlight: begin Go to C type marshaling
From: Nick Rosbrook
Implement conversion of basic type conversions such as strings
and integer types in toC functions.
Signed-off-by: Nick Rosbrook
---
tools/golang/xenlight/gengotypes.py | 80 ++
tools/golang/xenlight/helpers.gen.go | 1015 ++
2 files changed, 1095 insertions(+)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index ee9aaf9eff..cb9b18218f 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -234,6 +234,9 @@ def xenlight_golang_generate_helpers(path = None, types =
None, comment = None):
f.write(extra)
f.write('\n')
+f.write(xenlight_golang_define_to_C(ty))
+f.write('\n')
+
go_fmt(path)
def xenlight_golang_define_from_C(ty = None):
@@ -436,6 +439,83 @@ def xenlight_golang_array_from_C(ty = None):
return s
+def xenlight_golang_define_to_C(ty = None, typename = None, nested = False):
+s = ''
+
+gotypename = ctypename = ''
+
+if typename is not None:
+gotypename = xenlight_golang_fmt_name(typename)
+ctypename = typename
+else:
+gotypename = xenlight_golang_fmt_name(ty.typename)
+ctypename = ty.typename
+
+if not nested:
+s += 'func (x *{}) toC() (xc C.{},err error)
{{\n'.format(gotypename,ctypename)
+s += 'C.{}()\n'.format(ty.init_fn)
+
+for f in ty.fields:
+if f.type.typename is not None:
+if isinstance(f.type, idl.Array):
+# TODO
+continue
+
+gotypename = xenlight_golang_fmt_name(f.type.typename)
+ctypename = f.type.typename
+gofname= xenlight_golang_fmt_name(f.name)
+cfname = f.name
+
+# In cgo, C names that conflict with Go keywords can be
+# accessed by prepending an underscore to the name.
+if cfname in go_keywords:
+cfname = '_' + cfname
+
+# If this is nested, we need the outer name too.
+if nested and typename is not None:
+goname = xenlight_golang_fmt_name(typename)
+goname = '{}.{}'.format(goname, gofname)
+cname = '{}.{}'.format(typename, cfname)
+
+else:
+goname = gofname
+cname = cfname
+
+is_castable = (f.type.json_parse_type == 'JSON_INTEGER' or
+ isinstance(f.type, idl.Enumeration) or
+ gotypename in go_builtin_types)
+
+if is_castable:
+# Use the cgo helper for converting C strings.
+if gotypename == 'string':
+s += 'xc.{} = C.CString(x.{})\n'.format(cname,goname)
+continue
+
+s += 'xc.{} = C.{}(x.{})\n'.format(cname,ctypename,goname)
+
+else:
+s += 'xc.{}, err = x.{}.toC()\n'.format(cname,goname)
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(ty.dispose_fn)
+s += 'return xc, err\n'
+s += '}\n'
+
+elif isinstance(f.type, idl.Struct):
+s += xenlight_golang_define_to_C(f.type, typename=f.name,
nested=True)
+
+elif isinstance(f.type, idl.KeyedUnion):
+# TODO
+pass
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+if not nested:
+s += 'return xc, nil'
+s += '}\n'
+
+return s
+
def xenlight_golang_fmt_name(name, exported = True):
"""
Take a given type name and return an
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 2f917cac58..a155b091a7 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -37,6 +37,13 @@ func (x *IoportRange) fromC(xc *C.libxl_ioport_range) error {
return nil
}
+func (x *IoportRange) toC() (xc C.libxl_ioport_range, err error) {
+ C.libxl_ioport_range_init()
+ xc.first = C.uint32_t(x.First)
+ xc.number = C.uint32_t(x.Number)
+ return xc, nil
+}
+
func (x *IomemRange) fromC(xc *C.libxl_iomem_range) error {
x.Start = uint64(xc.start)
x.Number = uint64(xc.number)
@@ -45,12 +52,26 @@ func (x *IomemRange) fromC(xc *C.libxl_iomem_range) error {
return nil
}
+func (x *IomemRange) toC() (xc C.libxl_iomem_range, err error) {
+ C.libxl_iomem_range_init()
+ xc.start = C.uint64_t(x.Start)
+ xc.number = C.uint64_t(x.Number)
+ xc.gfn = C.uint64_t(x.Gfn)
+ return xc, nil
+}
+
func (x *VgaInterfaceInfo) fromC(xc *C.libxl_vga_interface_info) error {
x.Kind = VgaInterfaceType(xc.kind)
return nil
}
+func (x *VgaInterfaceInfo) toC() (xc C.libxl_vga_interface_info, err error) {
+ C.libxl_vga_interface_info_init()
+ xc.kind =
[Xen-devel] [PATCH v3 22/22] golang/xenlight: add error return type to Context.Cpupoolinfo
From: Nick Rosbrook
A previous commit that removed Context.CheckOpen revealed
an ineffectual assignent to err in Context.Cpupoolinfo, as
there is no error return type.
Since it appears that the intent is to return an error here,
add an error return value to the function signature.
Signed-off-by: Nick Rosbrook
---
tools/golang/xenlight/xenlight.go | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 1c431fa4e5..38d2ba8aa8 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -503,17 +503,17 @@ func (Ctx *Context) ListCpupool() (list []Cpupoolinfo) {
}
// int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t
poolid);
-func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo) {
+func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo, err error) {
var c_cpupool C.libxl_cpupoolinfo
ret := C.libxl_cpupool_info(Ctx.ctx, _cpupool, C.uint32_t(Poolid))
if ret != 0 {
- //err = Error(-ret)
+ err = Error(-ret)
return
}
defer C.libxl_cpupoolinfo_dispose(_cpupool)
- _ = pool.fromC(_cpupool)
+ err = pool.fromC(_cpupool)
return
}
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 14/22] golang/xenlight: remove no-longer used type MemKB
From: Nick Rosbrook Signed-off-by: Nick Rosbrook Acked-by: George Dunlap --- tools/golang/xenlight/xenlight.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go index 8f41047726..fb1c6d9e51 100644 --- a/tools/golang/xenlight/xenlight.go +++ b/tools/golang/xenlight/xenlight.go @@ -83,8 +83,6 @@ type Domid uint32 // Devid is a device ID. type Devid int -type MemKB uint64 - // Uuid is a domain UUID. type Uuid [16]byte -- 2.19.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 20/22] golang/xenlight: implement array Go to C marshaling
From: Nick Rosbrook
Signed-off-by: Nick Rosbrook
---
tools/golang/xenlight/gengotypes.py | 44 +++-
tools/golang/xenlight/helpers.gen.go | 359 +++
2 files changed, 402 insertions(+), 1 deletion(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 1839ac14b6..24d27e30ad 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -458,7 +458,7 @@ def xenlight_golang_define_to_C(ty = None, typename = None,
nested = False):
for f in ty.fields:
if f.type.typename is not None:
if isinstance(f.type, idl.Array):
-# TODO
+s += xenlight_golang_array_to_C(f, ty.dispose_fn)
continue
gotypename = xenlight_golang_fmt_name(f.type.typename)
@@ -589,6 +589,48 @@ def xenlight_golang_union_to_C(ty = None, union_name = '',
return s
+def xenlight_golang_array_to_C(ty = None, dispose_fn = ''):
+s = ''
+
+gotypename = xenlight_golang_fmt_name(ty.type.elem_type.typename)
+goname = xenlight_golang_fmt_name(ty.name)
+ctypename = ty.type.elem_type.typename
+cname = ty.name
+clenvar= ty.type.lenvar.name
+golenvar = xenlight_golang_fmt_name(clenvar,exported=False)
+
+is_enum = isinstance(ty.type.elem_type,idl.Enumeration)
+if gotypename in go_builtin_types or is_enum:
+s += '{} := len(x.{})\n'.format(golenvar,goname)
+s += 'xc.{} =
(*C.{})(C.malloc(C.size_t({}*{})))\n'.format(cname,ctypename,
+
golenvar,golenvar)
+s += 'xc.{} = C.int({})\n'.format(clenvar,golenvar)
+s += 'c{} :=
(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(goname,
+
ctypename,cname,
+
golenvar,golenvar)
+s += 'for i,v := range x.{} {{\n'.format(goname)
+s += 'c{}[i] = C.{}(v)\n'.format(goname,ctypename)
+s += '}\n'
+
+return s
+
+s += '{} := len(x.{})\n'.format(golenvar,goname)
+s += 'xc.{} =
(*C.{})(C.malloc(C.ulong({})*C.sizeof_{}))\n'.format(cname,ctypename,
+
golenvar,ctypename)
+s += 'xc.{} = C.int({})\n'.format(clenvar,golenvar)
+s += 'c{} :=
(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(goname,
+
ctypename,cname,
+
golenvar,golenvar)
+s += 'for i,v := range x.{} {{\n'.format(goname)
+s += 'tmp, err := v.toC()\n'
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,err\n}\n'
+s += 'c{}[i] = tmp\n'.format(goname)
+s += '}\n'
+
+return s
+
def xenlight_golang_fmt_name(name, exported = True):
"""
Take a given type name and return an
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 73ad5e9761..98b4bba4b2 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -545,6 +545,18 @@ func (x *VcpuSchedParams) fromC(xc
*C.libxl_vcpu_sched_params) error {
func (x *VcpuSchedParams) toC() (xc C.libxl_vcpu_sched_params, err error) {
C.libxl_vcpu_sched_params_init()
xc.sched = C.libxl_scheduler(x.Sched)
+ numVcpus := len(x.Vcpus)
+ xc.vcpus = (*C.libxl_sched_params)(C.malloc(C.ulong(numVcpus) *
C.sizeof_libxl_sched_params))
+ xc.num_vcpus = C.int(numVcpus)
+ cVcpus := (*[1 <<
28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
+ for i, v := range x.Vcpus {
+ tmp, err := v.toC()
+ if err != nil {
+ C.libxl_vcpu_sched_params_dispose()
+ return xc, err
+ }
+ cVcpus[i] = tmp
+ }
return xc, nil
}
@@ -593,6 +605,13 @@ func (x *VnodeInfo) fromC(xc *C.libxl_vnode_info) error {
func (x *VnodeInfo) toC() (xc C.libxl_vnode_info, err error) {
C.libxl_vnode_info_init()
xc.memkb = C.uint64_t(x.Memkb)
+ numDistances := len(x.Distances)
+ xc.distances = (*C.uint32_t)(C.malloc(C.size_t(numDistances *
numDistances)))
+ xc.num_distances = C.int(numDistances)
+ cDistances := (*[1 <<
28]C.uint32_t)(unsafe.Pointer(xc.distances))[:numDistances:numDistances]
+ for i, v := range x.Distances {
+ cDistances[i] = C.uint32_t(v)
+ }
xc.pnode = C.uint32_t(x.Pnode)
xc.vcpus, err = x.Vcpus.toC()
if err != nil {
@@ -946,6 +965,30 @@ func (x *DomainBuildInfo) toC() (xc
C.libxl_domain_build_info, err error) {
C.libxl_domain_build_info_dispose()
return xc, err
}
+
[Xen-devel] [PATCH v3 06/22] golang/xenlight: define StringList builtin type
From: Nick Rosbrook
Define StringList as []string an implement fromC and toC functions.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
Changes in v2:
- Define fromC with a pointer receiver since a newly-allocated slice
is being assigned to the StringList.
---
tools/golang/xenlight/xenlight.go | 29 +
1 file changed, 29 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 1c5e3c0cc7..72afc3cf14 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -212,6 +212,35 @@ type KeyValueList struct{}
func (kvl KeyValueList) fromC(ckvl *C.libxl_key_value_list) error {
return nil }
func (kvl KeyValueList) toC() (ckvl C.libxl_key_value_list, err error) {
return }
+// StringList represents a libxl_string_list.
+type StringList []string
+
+func (sl *StringList) fromC(csl *C.libxl_string_list) error {
+ size := int(C.libxl_string_list_length(csl))
+ list := (*[1 << 30]*C.char)(unsafe.Pointer(csl))[:size:size]
+
+ *sl = make([]string, size)
+
+ for i, v := range list {
+ (*sl)[i] = C.GoString(v)
+ }
+
+ return nil
+}
+
+func (sl StringList) toC() (C.libxl_string_list, error) {
+ var char *C.char
+ size := len(sl)
+ csl := (C.libxl_string_list)(C.malloc(C.ulong(size) *
C.ulong(unsafe.Sizeof(char
+ clist := (*[1 << 30]*C.char)(unsafe.Pointer(csl))[:size:size]
+
+ for i, v := range sl {
+ clist[i] = C.CString(v)
+ }
+
+ return csl, nil
+}
+
// Bitmap represents a libxl_bitmap.
//
// Implement the Go bitmap type such that the underlying data can
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 12/22] golang/xenlight: re-factor Hwcap type implementation
From: Nick Rosbrook
Re-define Hwcap as [8]uint32, and implement toC function. Also, re-name and
modify signature of toGo function to fromC.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
Changes in v2:
- Fix comment in fromC since an array is being used now, not a slice.
- Use a concise variable name instead of mapslice for the C array.
Changes in v3:
- In fromC, iterate over the indirect of hwcap instead of creating
a slice from the C type.
---
tools/golang/xenlight/xenlight.go | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index f9c2f84c81..b395963512 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -306,20 +306,25 @@ type Context struct {
logger *C.xentoollog_logger_stdiostream
}
-type Hwcap []C.uint32_t
+// Hwcap represents a libxl_hwcap.
+type Hwcap [8]uint32
-func (chwcap C.libxl_hwcap) toGo() (ghwcap Hwcap) {
- // Alloc a Go slice for the bytes
- size := 8
- ghwcap = make([]C.uint32_t, size)
+func (hwcap *Hwcap) fromC(chwcap *C.libxl_hwcap) error {
+ for i := range *hwcap {
+ hwcap[i] = uint32(chwcap[i])
+ }
- // Make a slice pointing to the C array
- mapslice := (*[1 <<
30]C.uint32_t)(unsafe.Pointer([0]))[:size:size]
+ return nil
+}
- // And copy the C array into the Go array
- copy(ghwcap, mapslice)
+func (hwcap *Hwcap) toC() (C.libxl_hwcap, error) {
+ var chwcap C.libxl_hwcap
- return
+ for i, v := range hwcap {
+ chwcap[i] = C.uint32_t(v)
+ }
+
+ return chwcap, nil
}
// KeyValueList represents a libxl_key_value_list.
@@ -442,7 +447,7 @@ func (cphys *C.libxl_physinfo) toGo() (physinfo *Physinfo) {
physinfo.SharingFreedPages = uint64(cphys.sharing_freed_pages)
physinfo.SharingUsedFrames = uint64(cphys.sharing_used_frames)
physinfo.NrNodes = uint32(cphys.nr_nodes)
- physinfo.HwCap = cphys.hw_cap.toGo()
+ physinfo.HwCap.fromC(_cap)
physinfo.CapHvm = bool(cphys.cap_hvm)
physinfo.CapHvmDirectio = bool(cphys.cap_hvm_directio)
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 16/22] golang/xenlight: implement keyed union C to Go marshaling
From: Nick Rosbrook
Switch over union key to determine how to populate 'union' in Go struct.
Since the unions of C types cannot be directly accessed in cgo, use a
typeof trick to typedef a struct in the cgo preamble that is analagous
to each inner struct of a keyed union. For example, to define a struct
for the hvm inner struct of libxl_domain_build_info, do:
typedef typeof(((struct libxl_domain_build_info *)NULL)->u.hvm)
libxl_domain_build_info_type_union_hvm;
Signed-off-by: Nick Rosbrook
---
Changes in v2:
- Do not use global variable for extra helper function definitions.
Instead, return a tuple which contains a list of extra helper
functions associated with the original.
Changes in v3:
- Use the new xenlight_golang_convert_from_C function for field copying/
type conversion.
- Use a typeof trick in the cgo preamble to define structs for the inner
struct of keyed unions, instead of explicitly defining the fields.
---
tools/golang/xenlight/gengotypes.py | 129 +-
tools/golang/xenlight/helpers.gen.go | 346 +++
2 files changed, 464 insertions(+), 11 deletions(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 1fe56179e2..b68c1aa66b 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -24,6 +24,10 @@ go_keywords = ['type', 'func']
go_builtin_types = ['bool', 'string', 'int', 'byte',
'uint16', 'uint32', 'uint64']
+# cgo preamble for xenlight_helpers.go, created during type generation and
+# written later.
+cgo_helpers_preamble = []
+
def xenlight_golang_generate_types(path = None, types = None, comment = None):
"""
Generate a .go file (types.gen.go by default)
@@ -124,7 +128,7 @@ def xenlight_golang_define_struct(ty = None, typename =
None, nested = False):
extras.extend(r[1])
elif isinstance(f.type, idl.KeyedUnion):
-r = xenlight_golang_define_union(f.type, ty.typename)
+r = xenlight_golang_define_union(f.type, ty.typename, f.name)
s += r[0]
extras.extend(r[1])
@@ -137,7 +141,7 @@ def xenlight_golang_define_struct(ty = None, typename =
None, nested = False):
return (s,extras)
-def xenlight_golang_define_union(ty = None, structname = ''):
+def xenlight_golang_define_union(ty = None, struct_name = '', union_name = ''):
"""
Generate the Go translation of a KeyedUnion.
@@ -149,7 +153,7 @@ def xenlight_golang_define_union(ty = None, structname =
''):
s = ''
extras = []
-interface_name = '{}_{}_union'.format(structname, ty.keyvar.name)
+interface_name = '{}_{}_union'.format(struct_name, ty.keyvar.name)
interface_name = xenlight_golang_fmt_name(interface_name, exported=False)
s += 'type {} interface {{\n'.format(interface_name)
@@ -163,11 +167,18 @@ def xenlight_golang_define_union(ty = None, structname =
''):
continue
# Define struct
-name = '{}_{}_union_{}'.format(structname, ty.keyvar.name, f.name)
+name = '{}_{}_union_{}'.format(struct_name, ty.keyvar.name, f.name)
r = xenlight_golang_define_struct(f.type, typename=name)
extras.append(r[0])
extras.extend(r[1])
+# This typeof trick ensures that the fields used in the cgo struct
+# used for marshaling are the same as the fields of the union in the
+# actual C type, and avoids re-defining all of those fields.
+s = 'typedef typeof(((struct {} *)NULL)->{}.{}){};'
+s = s.format(struct_name, union_name, f.name, name)
+cgo_helpers_preamble.append(s)
+
# Define function to implement 'union' interface
name = xenlight_golang_fmt_name(name)
s = 'func (x {}) is{}(){{}}\n'.format(name, interface_name)
@@ -195,6 +206,7 @@ def xenlight_golang_generate_helpers(path = None, types =
None, comment = None):
if comment is not None:
f.write(comment)
f.write('package xenlight\n')
+f.write('import (\n"unsafe"\n"errors"\n"fmt"\n)\n')
# Cgo preamble
f.write('/*\n')
@@ -203,15 +215,25 @@ def xenlight_golang_generate_helpers(path = None, types =
None, comment = None):
f.write('#include \n')
f.write('\n')
+for s in cgo_helpers_preamble:
+f.write(s)
+f.write('\n')
+
f.write('*/\nimport "C"\n')
for ty in types:
if not isinstance(ty, idl.Struct):
continue
-f.write(xenlight_golang_define_from_C(ty))
+(fdef, extras) = xenlight_golang_define_from_C(ty)
+
+f.write(fdef)
f.write('\n')
+for extra in extras:
+f.write(extra)
+f.write('\n')
+
go_fmt(path)
def xenlight_golang_define_from_C(ty = None):
@@ -225,6 +247,7 @@ def xenlight_golang_define_from_C(ty = None):
cname = ty.typename
[Xen-devel] [PATCH v3 15/22] golang/xenlight: begin C to Go type marshaling
From: Nick Rosbrook
Begin implementation of fromC marshaling functions for generated struct
types. This includes support for converting fields that are basic
primitive types such as string and integer types, nested anonymous
structs, nested libxl structs, and libxl built-in types.
This patch does not implement conversion of arrays or keyed unions.
Signed-off-by: Nick Rosbrook
---
Changes in v2:
- Add Makefile changes for helpers.gen.go.
- Re-generate helpers.gen.go to include libxl changes after rebase.
Changes in v3:
- Break out field copying/type conversion code into its own function
called xenlight_golang_convert_from_C to allow that code to be easily
re-used.
- Use consistent style for calling fromC on struct fields that require
it. Namely, do not use a temporary variable - call fromC directly on
the struct field.
---
tools/golang/xenlight/Makefile | 2 +
tools/golang/xenlight/gengotypes.py | 118
tools/golang/xenlight/helpers.gen.go | 901 +++
tools/golang/xenlight/xenlight.go| 111 +---
4 files changed, 1032 insertions(+), 100 deletions(-)
create mode 100644 tools/golang/xenlight/helpers.gen.go
diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
index 681f32c234..07b8896e5b 100644
--- a/tools/golang/xenlight/Makefile
+++ b/tools/golang/xenlight/Makefile
@@ -19,6 +19,7 @@ $(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: %.gen.go
$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
$(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
$(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+ $(INSTALL_DATA) helpers.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
%.gen.go: gengotypes.py $(XEN_ROOT)/tools/libxl/libxl_types.idl
$(XEN_ROOT)/tools/libxl/idl.py
XEN_ROOT=$(XEN_ROOT) $(PYTHON) gengotypes.py ../../libxl/libxl_types.idl
@@ -39,6 +40,7 @@ install: build
$(INSTALL_DIR) $(DESTDIR)$(GOXL_INSTALL_DIR)
$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)xenlight.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)types.gen.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
+ $(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)helpers.gen.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
.PHONY: uninstall
rm -rf $(DESTDIR)$(GOXL_INSTALL_DIR)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 8963b14eee..1fe56179e2 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -18,6 +18,12 @@ builtin_type_names = {
idl.uint64.typename: 'uint64',
}
+# Some go keywords that conflict with field names in libxl structs.
+go_keywords = ['type', 'func']
+
+go_builtin_types = ['bool', 'string', 'int', 'byte',
+'uint16', 'uint32', 'uint64']
+
def xenlight_golang_generate_types(path = None, types = None, comment = None):
"""
Generate a .go file (types.gen.go by default)
@@ -176,6 +182,116 @@ def xenlight_golang_define_union(ty = None, structname =
''):
return (s,extras)
+def xenlight_golang_generate_helpers(path = None, types = None, comment =
None):
+"""
+Generate a .go file (helpers.gen.go by default)
+that contains helper functions for marshaling between
+C and Go types.
+"""
+if path is None:
+path = 'helpers.gen.go'
+
+with open(path, 'w') as f:
+if comment is not None:
+f.write(comment)
+f.write('package xenlight\n')
+
+# Cgo preamble
+f.write('/*\n')
+f.write('#cgo LDFLAGS: -lxenlight\n')
+f.write('#include \n')
+f.write('#include \n')
+f.write('\n')
+
+f.write('*/\nimport "C"\n')
+
+for ty in types:
+if not isinstance(ty, idl.Struct):
+continue
+
+f.write(xenlight_golang_define_from_C(ty))
+f.write('\n')
+
+go_fmt(path)
+
+def xenlight_golang_define_from_C(ty = None):
+"""
+Define the fromC marshaling function for the type
+represented by ty.
+"""
+func = 'func (x *{}) fromC(xc *C.{}) error {{\n {} \n return nil}}\n'
+
+goname = xenlight_golang_fmt_name(ty.typename)
+cname = ty.typename
+
+body = ''
+
+for f in ty.fields:
+if f.type.typename is not None:
+if isinstance(f.type, idl.Array):
+# TODO
+continue
+
+body += xenlight_golang_convert_from_C(f)
+
+elif isinstance(f.type, idl.Struct):
+# Go through the fields of the anonymous nested struct.
+for nf in f.type.fields:
+body += xenlight_golang_convert_from_C(nf,outer_name=f.name)
+
+elif isinstance(f.type, idl.KeyedUnion):
+pass
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+return func.format(goname, cname, body)
+
+def xenlight_golang_convert_from_C(ty = None, outer_name = None):
+"""
+Returns a line of Go
[Xen-devel] [PATCH v3 13/22] golang/xenlight: generate structs from the IDL
From: Nick Rosbrook
Add struct and keyed union generation to gengotypes.py. For keyed unions,
use a method similar to gRPC's oneof to interpret C unions as Go types.
Meaning, for a given struct with a union field, generate a struct for
each sub-struct defined in the union. Then, define an interface of one
method which is implemented by each of the defined sub-structs. For
example:
type domainBuildInfoTypeUnion interface {
isdomainBuildInfoTypeUnion()
}
type DomainBuildInfoTypeUnionHvm struct {
// HVM-specific fields...
}
func (x DomainBuildInfoTypeUnionHvm) isdomainBuildInfoTypeUnion() {}
type DomainBuildInfoTypeUnionPv struct {
// PV-specific fields...
}
func (x DomainBuildInfoTypeUnionPv) isdomainBuildInfoTypeUnion() {}
type DomainBuildInfoTypeUnionPvh struct {
// PVH-specific fields...
}
func (x DomainBuildInfoTypeUnionPvh) isdomainBuildInfoTypeUnion() {}
Then, remove existing struct definitions in xenlight.go that conflict
with the generated types, and modify existing marshaling functions to
align with the new type definitions. Notably, drop "time" package since
fields of type time.Duration are now of type uint64.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
Changes in v2:
- Do not use global variables for extra type definitions. Instead,
return a tuple which includes a list of extra type definitions
associated with the original type.
- Re-generate types.gen.go to include changes to libxl after rebase.
---
tools/golang/xenlight/gengotypes.py | 119 +++-
tools/golang/xenlight/types.gen.go | 836
tools/golang/xenlight/xenlight.go | 123 +---
3 files changed, 966 insertions(+), 112 deletions(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 2211541547..8963b14eee 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -32,18 +32,32 @@ def xenlight_golang_generate_types(path = None, types =
None, comment = None):
f.write('package xenlight\n')
for ty in types:
-f.write(xenlight_golang_type_define(ty))
+(tdef, extras) = xenlight_golang_type_define(ty)
+
+f.write(tdef)
f.write('\n')
+# Append extra types
+for extra in extras:
+f.write(extra)
+f.write('\n')
+
go_fmt(path)
def xenlight_golang_type_define(ty = None):
-s = ''
+"""
+Generate the Go type definition of ty.
+Return a tuple that contains a string with the
+type definition, and a (potentially empty) list
+of extra definitions that are associated with
+this type.
+"""
if isinstance(ty, idl.Enumeration):
-s += xenlight_golang_define_enum(ty)
+return (xenlight_golang_define_enum(ty), [])
-return s
+elif isinstance(ty, idl.Aggregate):
+return xenlight_golang_define_struct(ty)
def xenlight_golang_define_enum(ty = None):
s = ''
@@ -65,6 +79,103 @@ def xenlight_golang_define_enum(ty = None):
return s
+def xenlight_golang_define_struct(ty = None, typename = None, nested = False):
+s = ''
+extras = []
+name = ''
+
+if typename is not None:
+name = xenlight_golang_fmt_name(typename)
+else:
+name = xenlight_golang_fmt_name(ty.typename)
+
+# Begin struct definition
+if nested:
+s += '{} struct {{\n'.format(name)
+else:
+s += 'type {} struct {{\n'.format(name)
+
+# Write struct fields
+for f in ty.fields:
+if f.type.typename is not None:
+if isinstance(f.type, idl.Array):
+typename = f.type.elem_type.typename
+typename = xenlight_golang_fmt_name(typename)
+name = xenlight_golang_fmt_name(f.name)
+
+s += '{} []{}\n'.format(name, typename)
+else:
+typename = f.type.typename
+typename = xenlight_golang_fmt_name(typename)
+name = xenlight_golang_fmt_name(f.name)
+
+s += '{} {}\n'.format(name, typename)
+
+elif isinstance(f.type, idl.Struct):
+r = xenlight_golang_define_struct(f.type, typename=f.name,
nested=True)
+
+s += r[0]
+extras.extend(r[1])
+
+elif isinstance(f.type, idl.KeyedUnion):
+r = xenlight_golang_define_union(f.type, ty.typename)
+
+s += r[0]
+extras.extend(r[1])
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+# End struct definition
+s += '}\n'
+
+return (s,extras)
+
+def xenlight_golang_define_union(ty = None, structname = ''):
+"""
+Generate the Go translation of a KeyedUnion.
+
+Define an unexported interface to be used as
+the type of the union. Then, define a struct
+for each field of the union which implements
+that interface.
+
[Xen-devel] [PATCH v3 17/22] golang/xenlight: implement array C to Go marshaling
From: Nick Rosbrook
Signed-off-by: Nick Rosbrook
---
tools/golang/xenlight/gengotypes.py | 39 +++-
tools/golang/xenlight/helpers.gen.go | 300 +++
2 files changed, 338 insertions(+), 1 deletion(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index b68c1aa66b..ee9aaf9eff 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -252,7 +252,7 @@ def xenlight_golang_define_from_C(ty = None):
for f in ty.fields:
if f.type.typename is not None:
if isinstance(f.type, idl.Array):
-# TODO
+body += xenlight_golang_array_from_C(f)
continue
body += xenlight_golang_convert_from_C(f)
@@ -399,6 +399,43 @@ def xenlight_golang_union_from_C(ty = None, union_name =
'', struct_name = ''):
return (s,extras)
+def xenlight_golang_array_from_C(ty = None):
+"""
+Convert C array to Go slice using the method
+described here:
+
+https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
+"""
+s = ''
+
+gotypename = xenlight_golang_fmt_name(ty.type.elem_type.typename)
+goname = xenlight_golang_fmt_name(ty.name)
+ctypename = ty.type.elem_type.typename
+cname = ty.name
+cslice = 'c{}'.format(goname)
+clenvar= ty.type.lenvar.name
+golenvar = xenlight_golang_fmt_name(clenvar,exported=False)
+
+s += '{} := int(xc.{})\n'.format(golenvar, clenvar)
+s += '{} := '.format(cslice)
+s +='(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(ctypename,
cname,
+golenvar,
golenvar)
+s += 'x.{} = make([]{}, {})\n'.format(goname, gotypename, golenvar)
+s += 'for i, v := range {} {{\n'.format(cslice)
+
+is_enum = isinstance(ty.type.elem_type,idl.Enumeration)
+if gotypename in go_builtin_types or is_enum:
+s += 'x.{}[i] = {}(v)\n'.format(goname, gotypename)
+else:
+s += 'var e {}\n'.format(gotypename)
+s += 'if err := e.fromC(); err != nil {\n'
+s += 'return err }\n'
+s += 'x.{}[i] = e\n'.format(goname)
+
+s += '}\n'
+
+return s
+
def xenlight_golang_fmt_name(name, exported = True):
"""
Take a given type name and return an
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index e6eee234c0..2f917cac58 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -263,6 +263,16 @@ func (x *SchedParams) fromC(xc *C.libxl_sched_params)
error {
func (x *VcpuSchedParams) fromC(xc *C.libxl_vcpu_sched_params) error {
x.Sched = Scheduler(xc.sched)
+ numVcpus := int(xc.num_vcpus)
+ cVcpus := (*[1 <<
28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
+ x.Vcpus = make([]SchedParams, numVcpus)
+ for i, v := range cVcpus {
+ var e SchedParams
+ if err := e.fromC(); err != nil {
+ return err
+ }
+ x.Vcpus[i] = e
+ }
return nil
}
@@ -282,6 +292,12 @@ func (x *DomainSchedParams) fromC(xc
*C.libxl_domain_sched_params) error {
func (x *VnodeInfo) fromC(xc *C.libxl_vnode_info) error {
x.Memkb = uint64(xc.memkb)
+ numDistances := int(xc.num_distances)
+ cDistances := (*[1 <<
28]C.uint32_t)(unsafe.Pointer(xc.distances))[:numDistances:numDistances]
+ x.Distances = make([]uint32, numDistances)
+ for i, v := range cDistances {
+ x.Distances[i] = uint32(v)
+ }
x.Pnode = uint32(xc.pnode)
if err := x.Vcpus.fromC(); err != nil {
return err
@@ -308,6 +324,26 @@ func (x *DomainBuildInfo) fromC(xc
*C.libxl_domain_build_info) error {
if err := x.Nodemap.fromC(); err != nil {
return err
}
+ numVcpuHardAffinity := int(xc.num_vcpu_hard_affinity)
+ cVcpuHardAffinity := (*[1 <<
28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_hard_affinity))[:numVcpuHardAffinity:numVcpuHardAffinity]
+ x.VcpuHardAffinity = make([]Bitmap, numVcpuHardAffinity)
+ for i, v := range cVcpuHardAffinity {
+ var e Bitmap
+ if err := e.fromC(); err != nil {
+ return err
+ }
+ x.VcpuHardAffinity[i] = e
+ }
+ numVcpuSoftAffinity := int(xc.num_vcpu_soft_affinity)
+ cVcpuSoftAffinity := (*[1 <<
28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_soft_affinity))[:numVcpuSoftAffinity:numVcpuSoftAffinity]
+ x.VcpuSoftAffinity = make([]Bitmap, numVcpuSoftAffinity)
+ for i, v := range cVcpuSoftAffinity {
+ var e Bitmap
+ if err := e.fromC(); err != nil {
+ return err
+ }
+ x.VcpuSoftAffinity[i] = e
+ }
if err :=
[Xen-devel] [PATCH v3 11/22] golang/xenlight: re-factor Uuid type implementation
From: Nick Rosbrook
Re-define Uuid as [16]byte and implement fromC, toC, and String functions.
Signed-off-by: Nick Rosbrook
---
Changes in v3:
- In fromC, iterate over the indirect of u instead of creating a
slice from the C type.
---
tools/golang/xenlight/xenlight.go | 35 +--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 6b87bf857d..f9c2f84c81 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -86,7 +86,38 @@ type Devid int
type MemKB uint64
-type Uuid C.libxl_uuid
+// Uuid is a domain UUID.
+type Uuid [16]byte
+
+// String formats a Uuid in the form "-xx-xx-xx-xx".
+func (u Uuid) String() string {
+ s := "%x%x%x%x-%x%x-%x%x-%x%x-%x%x%x%x%x%x"
+ opts := make([]interface{}, 16)
+
+ for i, v := range u {
+ opts[i] = v
+ }
+
+ return fmt.Sprintf(s, opts...)
+}
+
+func (u *Uuid) fromC(c *C.libxl_uuid) error {
+ for i := range *u {
+ u[i] = byte(c.uuid[i])
+ }
+
+ return nil
+}
+
+func (u *Uuid) toC() (C.libxl_uuid, error) {
+ var c C.libxl_uuid
+
+ for i, v := range u {
+ c.uuid[i] = C.uint8_t(v)
+ }
+
+ return c, nil
+}
// defboolVal represents a defbool value.
type defboolVal int
@@ -495,7 +526,7 @@ type Dominfo struct {
func (cdi *C.libxl_dominfo) toGo() (di *Dominfo) {
di = {}
- di.Uuid = Uuid(cdi.uuid)
+ di.Uuid.fromC()
di.Domid = Domid(cdi.domid)
di.Ssidref = uint32(cdi.ssidref)
di.SsidLabel = C.GoString(cdi.ssid_label)
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 05/22] golang/xenlight: re-name Bitmap marshaling functions
From: Nick Rosbrook
Re-name and modify signature of toGo function to fromC. The reason for
using 'fromC' rather than 'toGo' is that it is not a good idea to define
methods on the C types. Also, add error return type to Bitmap's toC function.
Finally, as code-cleanup, re-organize the Bitmap type's comments as per
Go conventions.
Signed-off-by: Nick Rosbrook
Acked-by: George Dunlap
--
Changes in v2:
- Use consistent variable naming for slice created from
libxl_bitmap.
---
tools/golang/xenlight/xenlight.go | 94 ---
1 file changed, 48 insertions(+), 46 deletions(-)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 3edff18471..1c5e3c0cc7 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -212,20 +212,48 @@ type KeyValueList struct{}
func (kvl KeyValueList) fromC(ckvl *C.libxl_key_value_list) error {
return nil }
func (kvl KeyValueList) toC() (ckvl C.libxl_key_value_list, err error) {
return }
-// typedef struct {
-// uint32_t size; /* number of bytes in map */
-// uint8_t *map;
-// } libxl_bitmap;
-
+// Bitmap represents a libxl_bitmap.
+//
// Implement the Go bitmap type such that the underlying data can
// easily be copied in and out. NB that we still have to do copies
// both directions, because cgo runtime restrictions forbid passing to
// a C function a pointer to a Go-allocated structure which contains a
// pointer.
type Bitmap struct {
+ // typedef struct {
+ // uint32_t size; /* number of bytes in map */
+ // uint8_t *map;
+ // } libxl_bitmap;
bitmap []C.uint8_t
}
+func (bm *Bitmap) fromC(cbm *C.libxl_bitmap) error {
+ // Alloc a Go slice for the bytes
+ size := int(cbm.size)
+ bm.bitmap = make([]C.uint8_t, size)
+
+ // Make a slice pointing to the C array
+ cs := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+ // And copy the C array into the Go array
+ copy(bm.bitmap, cs)
+
+ return nil
+}
+
+func (bm *Bitmap) toC() (C.libxl_bitmap, error) {
+ var cbm C.libxl_bitmap
+
+ size := len(bm.bitmap)
+ cbm.size = C.uint32_t(size)
+ cbm._map = (*C.uint8_t)(C.malloc(C.ulong(cbm.size) * C.sizeof_uint8_t))
+ cs := (*[1 << 31]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+ copy(cs, bm.bitmap)
+
+ return cbm, nil
+}
+
/*
* Types: IDL
*
@@ -426,7 +454,7 @@ func (cci C.libxl_cpupoolinfo) toGo() (gci CpupoolInfo) {
gci.PoolName = C.GoString(cci.pool_name)
gci.Scheduler = Scheduler(cci.sched)
gci.DomainCount = int(cci.n_dom)
- gci.Cpumap = cci.cpumap.toGo()
+ gci.Cpumap.fromC()
return
}
@@ -500,7 +528,10 @@ func (Ctx *Context) CpupoolCreate(Name string, Scheduler
Scheduler, Cpumap Bitma
var uuid C.libxl_uuid
C.libxl_uuid_generate()
- cbm := Cpumap.toC()
+ cbm, err := Cpumap.toC()
+ if err != nil {
+ return
+ }
defer C.libxl_bitmap_dispose()
ret := C.libxl_cpupool_create(Ctx.ctx, name,
C.libxl_scheduler(Scheduler),
@@ -555,7 +586,10 @@ func (Ctx *Context) CpupoolCpuaddCpumap(Poolid uint32,
Cpumap Bitmap) (err error
return
}
- cbm := Cpumap.toC()
+ cbm, err := Cpumap.toC()
+ if err != nil {
+ return
+ }
defer C.libxl_bitmap_dispose()
ret := C.libxl_cpupool_cpuadd_cpumap(Ctx.ctx, C.uint32_t(Poolid), )
@@ -591,7 +625,10 @@ func (Ctx *Context) CpupoolCpuremoveCpumap(Poolid uint32,
Cpumap Bitmap) (err er
return
}
- cbm := Cpumap.toC()
+ cbm, err := Cpumap.toC()
+ if err != nil {
+ return
+ }
defer C.libxl_bitmap_dispose()
ret := C.libxl_cpupool_cpuremove_cpumap(Ctx.ctx, C.uint32_t(Poolid),
)
@@ -714,41 +751,6 @@ func (Ctx *Context) CpupoolMakeFree(Cpumap Bitmap) (err
error) {
* Bitmap operations
*/
-// Return a Go bitmap which is a copy of the referred C bitmap.
-func (cbm C.libxl_bitmap) toGo() (gbm Bitmap) {
- // Alloc a Go slice for the bytes
- size := int(cbm.size)
- gbm.bitmap = make([]C.uint8_t, size)
-
- // Make a slice pointing to the C array
- mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
-
- // And copy the C array into the Go array
- copy(gbm.bitmap, mapslice)
-
- return
-}
-
-// Must be C.libxl_bitmap_dispose'd of afterwards
-func (gbm Bitmap) toC() (cbm C.libxl_bitmap) {
- C.libxl_bitmap_init()
-
- size := len(gbm.bitmap)
- cbm._map = (*C.uint8_t)(C.malloc(C.size_t(size)))
- cbm.size = C.uint32_t(size)
- if cbm._map == nil {
- panic("C.calloc failed!")
- }
-
- // Make a slice pointing to the C array
- mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
[Xen-devel] [PATCH v3 10/22] golang/xenlight: define CpuidPolicyList builtin type
From: Nick Rosbrook
Define CpuidPolicyList as a string so that libxl_cpuid_parse_config can
be used in the toC function.
For now, fromC is a no-op since libxl does not support a way to read a
policy, modify it,and then give it back to libxl.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
Changes in v2:
- Re-define CpuidPolicyList as string.
- Make fromC a no-op.
- Use libxl_cpuid_parse_config in toC function.
---
tools/golang/xenlight/xenlight.go | 25 +
1 file changed, 25 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index c1d9fe85fd..6b87bf857d 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -245,6 +245,31 @@ type EvLink struct{}
func (el *EvLink) fromC(cel *C.libxl_ev_link) error { return nil }
func (el *EvLink) toC() (cel C.libxl_ev_link, err error) { return }
+// CpuidPolicyList represents a libxl_cpuid_policy_list.
+//
+// The value of CpuidPolicyList is honored when used as input to libxl. If
+// a struct contains a field of type CpuidPolicyList, that field will be left
+// empty when it is returned from libxl.
+type CpuidPolicyList string
+
+func (cpl CpuidPolicyList) fromC(ccpl *C.libxl_cpuid_policy_list) error {
return nil }
+
+func (cpl CpuidPolicyList) toC() (C.libxl_cpuid_policy_list, error) {
+ var ccpl C.libxl_cpuid_policy_list
+
+ s := C.CString(string(cpl))
+ defer C.free(unsafe.Pointer(s))
+
+ ret := C.libxl_cpuid_parse_config(, s)
+ if ret != 0 {
+ C.libxl_cpuid_dispose()
+
+ return ccpl, Error(-ret)
+ }
+
+ return ccpl, nil
+}
+
type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 09/22] golang/xenlight: define EvLink builtin as empty struct
From: Nick Rosbrook
Define EvLink as empty struct as there is currently no reason the internal of
this type should be used in Go.
Implement fromC and toC functions as no-ops.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
tools/golang/xenlight/xenlight.go | 10 ++
1 file changed, 10 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 6c3868cd69..c1d9fe85fd 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -235,6 +235,16 @@ func (mvg *MsVmGenid) toC() (C.libxl_ms_vm_genid, error) {
return cmvg, nil
}
+// EvLink represents a libxl_ev_link.
+//
+// Represented as an empty struct for now, as there is no
+// apparent need for the internals of this type to be exposed
+// through the Go package.
+type EvLink struct{}
+
+func (el *EvLink) fromC(cel *C.libxl_ev_link) error { return nil }
+func (el *EvLink) toC() (cel C.libxl_ev_link, err error) { return }
+
type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 03/22] golang/xenlight: define Devid type as int
From: Nick Rosbrook
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
tools/golang/xenlight/xenlight.go | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 640d82f35f..8ac26e63f0 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -81,6 +81,9 @@ func (e Error) Error() string {
type Domid uint32
+// Devid is a device ID.
+type Devid int
+
type MemKB uint64
type Uuid C.libxl_uuid
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 01/22] golang/xenlight: generate enum types from IDL
From: Nick Rosbrook
Introduce gengotypes.py to generate Go code the from IDL. As a first step,
implement 'enum' type generation.
As a result of the newly-generated code, remove the existing, and now
conflicting definitions in xenlight.go. In the case of the Error type,
rename the slice 'errors' to 'libxlErrors' so that it does not conflict
with the standard library package 'errors.' And, negate the values used
in 'libxlErrors' since the generated error values are negative.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
Changes in v2:
- Introduce Makefile targets for code generation
- Re-generate Go code (includes new libxl_passtrhough enum).
- Use *.gen.go naming convention for generated Go files.
---
tools/golang/xenlight/Makefile | 18 +-
tools/golang/xenlight/gengotypes.py | 109
tools/golang/xenlight/types.gen.go | 388
tools/golang/xenlight/xenlight.go | 140 ++
4 files changed, 535 insertions(+), 120 deletions(-)
create mode 100644 tools/golang/xenlight/gengotypes.py
create mode 100644 tools/golang/xenlight/types.gen.go
diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
index 0987305224..681f32c234 100644
--- a/tools/golang/xenlight/Makefile
+++ b/tools/golang/xenlight/Makefile
@@ -7,20 +7,21 @@ GOCODE_DIR ?= $(prefix)/share/gocode/
GOXL_PKG_DIR = /src/$(XEN_GOCODE_URL)/xenlight/
GOXL_INSTALL_DIR = $(GOCODE_DIR)$(GOXL_PKG_DIR)
-# PKGSOURCES: Files which comprise the distributed source package
-PKGSOURCES = xenlight.go
-
GO ?= go
.PHONY: all
all: build
.PHONY: package
-package: $(XEN_GOPATH)$(GOXL_PKG_DIR)$(PKGSOURCES)
+package: $(XEN_GOPATH)$(GOXL_PKG_DIR)
-$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/$(PKGSOURCES): $(PKGSOURCES)
+$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: %.gen.go
$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
- $(INSTALL_DATA) $(PKGSOURCES) $(XEN_GOPATH)$(GOXL_PKG_DIR)
+ $(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+ $(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+
+%.gen.go: gengotypes.py $(XEN_ROOT)/tools/libxl/libxl_types.idl
$(XEN_ROOT)/tools/libxl/idl.py
+ XEN_ROOT=$(XEN_ROOT) $(PYTHON) gengotypes.py ../../libxl/libxl_types.idl
# Go will do its own dependency checking, and not actuall go through
# with the build if none of the input files have changed.
@@ -36,10 +37,11 @@ build: package
.PHONY: install
install: build
$(INSTALL_DIR) $(DESTDIR)$(GOXL_INSTALL_DIR)
- $(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)$(PKGSOURCES)
$(DESTDIR)$(GOXL_INSTALL_DIR)
+ $(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)xenlight.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
+ $(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)types.gen.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
.PHONY: uninstall
- rm -f $(addprefix $(DESTDIR)$(GOXL_INSTALL_DIR)/, $(PKGSOURCES))
+ rm -rf $(DESTDIR)$(GOXL_INSTALL_DIR)
.PHONY: clean
clean:
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
new file mode 100644
index 00..2211541547
--- /dev/null
+++ b/tools/golang/xenlight/gengotypes.py
@@ -0,0 +1,109 @@
+#!/usr/bin/python
+
+import os
+import sys
+
+sys.path.append('{}/tools/libxl'.format(os.environ['XEN_ROOT']))
+import idl
+
+# Go versions of some builtin types.
+# Append the libxl-defined builtins after IDL parsing.
+builtin_type_names = {
+idl.bool.typename: 'bool',
+idl.string.typename: 'string',
+idl.integer.typename: 'int',
+idl.uint8.typename: 'byte',
+idl.uint16.typename: 'uint16',
+idl.uint32.typename: 'uint32',
+idl.uint64.typename: 'uint64',
+}
+
+def xenlight_golang_generate_types(path = None, types = None, comment = None):
+"""
+Generate a .go file (types.gen.go by default)
+that contains a Go type for each type in types.
+"""
+if path is None:
+path = 'types.gen.go'
+
+with open(path, 'w') as f:
+if comment is not None:
+f.write(comment)
+f.write('package xenlight\n')
+
+for ty in types:
+f.write(xenlight_golang_type_define(ty))
+f.write('\n')
+
+go_fmt(path)
+
+def xenlight_golang_type_define(ty = None):
+s = ''
+
+if isinstance(ty, idl.Enumeration):
+s += xenlight_golang_define_enum(ty)
+
+return s
+
+def xenlight_golang_define_enum(ty = None):
+s = ''
+typename = ''
+
+if ty.typename is not None:
+typename = xenlight_golang_fmt_name(ty.typename)
+s += 'type {} int\n'.format(typename)
+
+# Start const block
+s += 'const(\n'
+
+for v in ty.values:
+name = xenlight_golang_fmt_name(v.name)
+s += '{} {} = {}\n'.format(name, typename, v.value)
+
+# End const block
+s += ')\n'
+
+return s
+
+def xenlight_golang_fmt_name(name, exported = True):
+"""
+Take a given type name and return an
+appropriate Go type name.
+"""
+if name in
[Xen-devel] [PATCH v3 08/22] golang/xenlight: define MsVmGenid builtin type
From: Nick Rosbrook
Define MsVmGenid as [int(C.LIBXL_MS_VM_GENID_LEN)]byte and implement fromC and
toC functions.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
Changes in v3:
- In fromC, iterate over the indirect of mvg instead of creating
a slice from the C type.
---
tools/golang/xenlight/xenlight.go | 21 +
1 file changed, 21 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 17d146771e..6c3868cd69 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -214,6 +214,27 @@ func (mac Mac) toC() (C.libxl_mac, error) {
return cmac, nil
}
+// MsVmGenid represents a libxl_ms_vm_genid.
+type MsVmGenid [int(C.LIBXL_MS_VM_GENID_LEN)]byte
+
+func (mvg *MsVmGenid) fromC(cmvg *C.libxl_ms_vm_genid) error {
+ for i := range *mvg {
+ mvg[i] = byte(cmvg.bytes[i])
+ }
+
+ return nil
+}
+
+func (mvg *MsVmGenid) toC() (C.libxl_ms_vm_genid, error) {
+ var cmvg C.libxl_ms_vm_genid
+
+ for i, v := range mvg {
+ cmvg.bytes[i] = C.uint8_t(v)
+ }
+
+ return cmvg, nil
+}
+
type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 07/22] golang/xenlight: define Mac builtin type
From: Nick Rosbrook
Define Mac as [6]byte and implement fromC, toC, and String functions.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
Changes in v2:
- Fix the format string in String function to use %02x.
- Use a value reciever for the toC function.
Changes in v3:
- Iterate over the indirect of mac instead of creating
a slice from the C type.
---
tools/golang/xenlight/xenlight.go | 33 +++
1 file changed, 33 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 72afc3cf14..17d146771e 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -181,6 +181,39 @@ func (d *Defbool) toC() (C.libxl_defbool, error) {
return c, nil
}
+// Mac represents a libxl_mac, or simply a MAC address.
+type Mac [6]byte
+
+// String formats a Mac address to string representation.
+func (mac Mac) String() string {
+ s := "%02x:%02x:%02x:%02x:%02x:%02x"
+ opts := make([]interface{}, 6)
+
+ for i, v := range mac {
+ opts[i] = v
+ }
+
+ return fmt.Sprintf(s, opts...)
+}
+
+func (mac *Mac) fromC(cmac *C.libxl_mac) error {
+ for i := range *mac {
+ mac[i] = byte(cmac[i])
+ }
+
+ return nil
+}
+
+func (mac Mac) toC() (C.libxl_mac, error) {
+ var cmac C.libxl_mac
+
+ for i, v := range mac {
+ cmac[i] = C.uint8_t(v)
+ }
+
+ return cmac, nil
+}
+
type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 02/22] golang/xenlight: define Defbool builtin type
From: Nick Rosbrook
Define Defbool as struct analagous to the C type, and define the type
'defboolVal' that represent true, false, and default defbool values.
Implement Set, Unset, SetIfDefault, IsDefault, Val, and String functions
on Defbool so that the type can be used in Go analagously to how its
used in C.
Finally, implement fromC and toC functions.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
tools/golang/xenlight/xenlight.go | 93 +++
1 file changed, 93 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 89ed439fd0..640d82f35f 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -85,6 +85,99 @@ type MemKB uint64
type Uuid C.libxl_uuid
+// defboolVal represents a defbool value.
+type defboolVal int
+
+const (
+ defboolDefault defboolVal = 0
+ defboolFalse defboolVal = -1
+ defboolTruedefboolVal = 1
+)
+
+// Defbool represents a libxl_defbool.
+type Defbool struct {
+ val defboolVal
+}
+
+func (d Defbool) String() string {
+ switch d.val {
+ case defboolDefault:
+ return ""
+ case defboolFalse:
+ return "False"
+ case defboolTrue:
+ return "True"
+ }
+
+ return ""
+}
+
+// Set sets the value of the Defbool.
+func (d *Defbool) Set(b bool) {
+ if b {
+ d.val = defboolTrue
+ return
+ }
+ d.val = defboolFalse
+}
+
+// Unset resets the Defbool to default value.
+func (d *Defbool) Unset() {
+ d.val = defboolDefault
+}
+
+// SetIfDefault sets the value of Defbool only if
+// its current value is default.
+func (d *Defbool) SetIfDefault(b bool) {
+ if d.IsDefault() {
+ d.Set(b)
+ }
+}
+
+// IsDefault returns true if the value of Defbool
+// is default, returns false otherwise.
+func (d *Defbool) IsDefault() bool {
+ return d.val == defboolDefault
+}
+
+// Val returns the boolean value associated with the
+// Defbool value. An error is returned if the value
+// is default.
+func (d *Defbool) Val() (bool, error) {
+ if d.IsDefault() {
+ return false, fmt.Errorf("%v: cannot take value of default
defbool", ErrorInval)
+ }
+
+ return (d.val > 0), nil
+}
+
+func (d *Defbool) fromC(c *C.libxl_defbool) error {
+ if C.libxl_defbool_is_default(*c) {
+ d.val = defboolDefault
+ return nil
+ }
+
+ if C.libxl_defbool_val(*c) {
+ d.val = defboolTrue
+ return nil
+ }
+
+ d.val = defboolFalse
+
+ return nil
+}
+
+func (d *Defbool) toC() (C.libxl_defbool, error) {
+ var c C.libxl_defbool
+
+ if !d.IsDefault() {
+ val, _ := d.Val()
+ C.libxl_defbool_set(, C.bool(val))
+ }
+
+ return c, nil
+}
+
type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 04/22] golang/xenlight: define KeyValueList as empty struct
From: Nick Rosbrook
Define KeyValueList as empty struct as there is currently no reason for
this type to be available in the Go package.
Implement fromC and toC functions as no-ops.
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
Changes in v2:
- Re-define KeyValueList as empty struct, as it was decided this type
probably shouldn't be exposed in the Go package.
---
tools/golang/xenlight/xenlight.go | 10 ++
1 file changed, 10 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 8ac26e63f0..3edff18471 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -202,6 +202,16 @@ func (chwcap C.libxl_hwcap) toGo() (ghwcap Hwcap) {
return
}
+// KeyValueList represents a libxl_key_value_list.
+//
+// Represented as an empty struct for now, as there is no
+// apparent need for this type to be exposed through the
+// Go package.
+type KeyValueList struct{}
+
+func (kvl KeyValueList) fromC(ckvl *C.libxl_key_value_list) error {
return nil }
+func (kvl KeyValueList) toC() (ckvl C.libxl_key_value_list, err error) {
return }
+
// typedef struct {
// uint32_t size; /* number of bytes in map */
// uint8_t *map;
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 00/22] generated Go libxl bindings using IDL
From: Nick Rosbrook After Xen summit, we started the discussion in this[1] RFC thread to figure out how to generate Go bindings for libxl. This series implements that Go code generation using the existing IDL, and updates the existing hand-written code in xenlight.go to use the generated code. The goal of this series is to provide a good foundation for continued development of the Go package. The v1 series can be found on my GitHub branch[2]. Changes in v2: - GitHub branch for v2 [3]. - Drop patch 01/24 from v1 since was committed as a bug fix for 4.13. - The Makefile changes in 24/24 from v1 have been moved to the patches where the build changes are introduced. Changes in v3: - GitHub branch for v3 [4]. - Simplify a pattern for iterating over builtin types in their fromC functions. - Try not to duplicate as much code in gengotypes.py, and use consistent style in generated code when calling fromC. [1] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02259.html [2] https://github.com/enr0n/xen/tree/golang-patches-v1 [3] https://github.com/enr0n/xen/tree/golang-patches-v2 [4] https://github.com/enr0n/xen/tree/golang-patches-v3 Nick Rosbrook (22): golang/xenlight: generate enum types from IDL golang/xenlight: define Defbool builtin type golang/xenlight: define Devid type as int golang/xenlight: define KeyValueList as empty struct golang/xenlight: re-name Bitmap marshaling functions golang/xenlight: define StringList builtin type golang/xenlight: define Mac builtin type golang/xenlight: define MsVmGenid builtin type golang/xenlight: define EvLink builtin as empty struct golang/xenlight: define CpuidPolicyList builtin type golang/xenlight: re-factor Uuid type implementation golang/xenlight: re-factor Hwcap type implementation golang/xenlight: generate structs from the IDL golang/xenlight: remove no-longer used type MemKB golang/xenlight: begin C to Go type marshaling golang/xenlight: implement keyed union C to Go marshaling golang/xenlight: implement array C to Go marshaling golang/xenlight: begin Go to C type marshaling golang/xenlight: implement keyed union Go to C marshaling golang/xenlight: implement array Go to C marshaling golang/xenlight: revise use of Context type golang/xenlight: add error return type to Context.Cpupoolinfo tools/golang/xenlight/Makefile | 20 +- tools/golang/xenlight/gengotypes.py | 677 ++ tools/golang/xenlight/helpers.gen.go | 3246 ++ tools/golang/xenlight/types.gen.go | 1224 ++ tools/golang/xenlight/xenlight.go| 901 +++ 5 files changed, 5531 insertions(+), 537 deletions(-) create mode 100644 tools/golang/xenlight/gengotypes.py create mode 100644 tools/golang/xenlight/helpers.gen.go create mode 100644 tools/golang/xenlight/types.gen.go -- 2.19.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 1/7] x86: import hyperv-tlfs.h from Linux
On 10.12.2019 16:37, Durrant, Paul wrote:
>> -Original Message-
>> From: Xen-devel On Behalf Of Jan
>> Beulich
>> Sent: 10 December 2019 15:34
>> To: Wei Liu
>> Cc: Wei Liu ; Paul Durrant ; Andrew
>> Cooper ; Michael Kelley
>> ; Xen Development List > de...@lists.xenproject.org>; Roger Pau Monné
>> Subject: Re: [Xen-devel] [PATCH for-next 1/7] x86: import hyperv-tlfs.h
>> from Linux
>>
>> On 25.10.2019 11:16, Wei Liu wrote:
>>> Taken from Linux commit b2d8b167e15bb5ec2691d1119c025630a247f649.
>>>
>>> This is a pristine copy from Linux. It is not used yet and probably
>>> doesn't compile. Changes to make it work will come later.
>>>
>>> Signed-off-by: Wei Liu
>>
>> This coming from Linux and assuming at least a fair part of it is
>> going to be used, in principle
>> Acked-by: Jan Beulich
>>
>> However, there are many seemingly unnecessary uses of __packed
>> here, which I'd rather not see go in at all (i.e. not be dropped
>> later on, and then potentially missing some). I find ...
>>
>>> +typedef struct _HV_REFERENCE_TSC_PAGE {
>>> + __u32 tsc_sequence;
>>> + __u32 res1;
>>> + __u64 tsc_scale;
>>> + __s64 tsc_offset;
>>> +} __packed HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE;
>>
>
> You realise there's a definition of this in the viridian code already, right?
It looked familiar, but it didn't occur to me to point this out.
Yes, there looks to be room for deduplication...
Actually, Wei, one more thing I was curious about - what is "tlfs"
an acronym of?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 1/7] x86: import hyperv-tlfs.h from Linux
> -Original Message-
> From: Xen-devel On Behalf Of Jan
> Beulich
> Sent: 10 December 2019 15:34
> To: Wei Liu
> Cc: Wei Liu ; Paul Durrant ; Andrew
> Cooper ; Michael Kelley
> ; Xen Development List de...@lists.xenproject.org>; Roger Pau Monné
> Subject: Re: [Xen-devel] [PATCH for-next 1/7] x86: import hyperv-tlfs.h
> from Linux
>
> On 25.10.2019 11:16, Wei Liu wrote:
> > Taken from Linux commit b2d8b167e15bb5ec2691d1119c025630a247f649.
> >
> > This is a pristine copy from Linux. It is not used yet and probably
> > doesn't compile. Changes to make it work will come later.
> >
> > Signed-off-by: Wei Liu
>
> This coming from Linux and assuming at least a fair part of it is
> going to be used, in principle
> Acked-by: Jan Beulich
>
> However, there are many seemingly unnecessary uses of __packed
> here, which I'd rather not see go in at all (i.e. not be dropped
> later on, and then potentially missing some). I find ...
>
> > +typedef struct _HV_REFERENCE_TSC_PAGE {
> > + __u32 tsc_sequence;
> > + __u32 res1;
> > + __u64 tsc_scale;
> > + __s64 tsc_offset;
> > +} __packed HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE;
>
You realise there's a definition of this in the viridian code already, right?
Paul
> .. this one particularly suspicious: I don't think it is well
> defined for __packed to also apply to the type
> PHV_REFERENCE_TSC_PAGE points to (and I suspect it doesn't).
>
> Jan
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 2/7] x86: fix up hyperv-tlfs.h
On 25.10.2019 11:16, Wei Liu wrote: > Do the following: > 1. include xen/types.h and xen/bitops.h > 2. fix up invocations of BIT macro Is it truly BIT(..., UL) in _all_ cases, and not BIT(..., U) in some? > Signed-off-by: Wei Liu > --- > This can be squashed into previous patch if preferred. Afaic - yes please. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-next 1/7] x86: import hyperv-tlfs.h from Linux
On 25.10.2019 11:16, Wei Liu wrote:
> Taken from Linux commit b2d8b167e15bb5ec2691d1119c025630a247f649.
>
> This is a pristine copy from Linux. It is not used yet and probably
> doesn't compile. Changes to make it work will come later.
>
> Signed-off-by: Wei Liu
This coming from Linux and assuming at least a fair part of it is
going to be used, in principle
Acked-by: Jan Beulich
However, there are many seemingly unnecessary uses of __packed
here, which I'd rather not see go in at all (i.e. not be dropped
later on, and then potentially missing some). I find ...
> +typedef struct _HV_REFERENCE_TSC_PAGE {
> + __u32 tsc_sequence;
> + __u32 res1;
> + __u64 tsc_scale;
> + __s64 tsc_offset;
> +} __packed HV_REFERENCE_TSC_PAGE, *PHV_REFERENCE_TSC_PAGE;
.. this one particularly suspicious: I don't think it is well
defined for __packed to also apply to the type
PHV_REFERENCE_TSC_PAGE points to (and I suspect it doesn't).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v4 1/3] xen/flask: Drop the gen-policy.py script
Hi, On 10/12/2019 12:17, Andrew Cooper wrote: The script is Python 2 specific, and fails with string/binary issues with Python 3: Traceback (most recent call last): File "gen-policy.py", line 14, in for char in sys.stdin.read(): File "/usr/lib/python3.5/codecs.py", line 321, in decode (result, consumed) = self._buffer_decode(data, self.errors, final) UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte Fixing the script to be compatible isn't hard, but using python here is wasteful. Drop the script entirely, and write an equivelent flask-policy.S instead. This removes the need for a $(PYTHON) and $(CC) pass. Signed-off-by: Andrew Cooper Acked-by: Daniel De Graaf --- CC: Daniel De Graaf CC: Juergen Gross CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné CC: Stefano Stabellini CC: Julien Grall CC: Volodymyr Babchuk v2: * Fix tabs vs spaces issues v3: * Use % rather than @ for progbits/object, for Arm32 build. How to make developper life more exciting... Acked-by: Julien Grall Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] x86/mm: factor out the code for shattering an l2 PTE
On 09.12.2019 12:48, Hongyan Xia wrote: > map_pages_to_xen and modify_xen_mappings are performing almost exactly > the same operations when shattering an l2 PTE, the only difference > being whether we want to flush. > > Signed-off-by: Hongyan Xia Mostly the same comments as for patch 1 (I think one is inapplicable here). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x86/mm: factor out the code for shattering an l3 PTE
On 09.12.2019 12:48, Hongyan Xia wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5151,6 +5151,51 @@ l1_pgentry_t *virt_to_xen_l1e(unsigned long v)
> flush_area_local((const void *)v, f) : \
> flush_area_all((const void *)v, f))
>
> +/* Shatter an l3 entry and populate l2. If virt is passed in, also do flush.
> */
> +static int shatter_l3e(l3_pgentry_t *pl3e, unsigned long virt, bool locking)
> +{
> +unsigned int i;
> +l3_pgentry_t ol3e;
> +l2_pgentry_t ol2e, *l2t = alloc_xen_pagetable();
> +
> +if ( l2t == NULL )
Nowadays we seem to prefer !l2t in cases like this one.
> +return -1;
-ENOMEM please (and then handed on by the caller).
> +ol3e = *pl3e;
This could be the variable's initializer.
> +ol2e = l2e_from_intpte(l3e_get_intpte(ol3e));
There's nothing "old" about this L2 entry, so its name would better
be just "l2e" I think.
> +for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
> +{
> +l2e_write(l2t + i, ol2e);
> +ol2e = l2e_from_intpte(
> +l2e_get_intpte(ol2e) + (1 << (PAGETABLE_ORDER +
> PAGE_SHIFT)));
Indentation looks odd here (also further down). If the first argument
of a function call doesn't fit on the line and would also be ugly to
split across lines, what we do is indent it the usual 4 characters
from the function invocation, i.e. in this case
ol2e = l2e_from_intpte(
l2e_get_intpte(ol2e) + (1 << (PAGETABLE_ORDER +
PAGE_SHIFT)));
and then slightly shorter
ol2e = l2e_from_intpte(
l2e_get_intpte(ol2e) + (PAGE_SIZE << PAGETABLE_ORDER));
Of course, as mentioned before, I'm not overly happy to see type
safety lost in case like this one, where it's not needed like e.g.
further up to convert from L3 to L2 entry.
> +}
> +if ( locking )
> +spin_lock(_pgdir_lock);
> +if ( (l3e_get_flags(*pl3e) & _PAGE_PRESENT) &&
> + (l3e_get_flags(*pl3e) & _PAGE_PSE) )
> +{
> +l3e_write_atomic(pl3e,
> +l3e_from_paddr((paddr_t)virt_to_maddr(l2t),
> __PAGE_HYPERVISOR));
> +l2t = NULL;
> +}
> +if ( locking )
> +spin_unlock(_pgdir_lock);
> +if ( virt )
> +{
> +unsigned int flush_flags =
> +FLUSH_TLB | FLUSH_ORDER(2 * PAGETABLE_ORDER);
> +
> +if ( (l3e_get_flags(ol3e) & _PAGE_GLOBAL) )
Unnecessary pair of parentheses (which also wasn't there in the
original code).
> +flush_flags |= FLUSH_TLB_GLOBAL;
Too deep indentation.
> +flush_area(virt, flush_flags);
> +}
> +if ( l2t )
> +free_xen_pagetable(l2t);
> +
> +return 0;
> +}
Also please add blank lines between
- L2 population and lock acquire,
- lock release and TLB flush,
- TLB flush and free.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 144647: trouble: broken/fail/pass
flight 144647 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/144647/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-freebsd10-amd64 broken test-amd64-i386-xl-qemuu-win7-amd64 broken test-amd64-i386-xl-qemut-debianhvm-amd64broken test-amd64-i386-xl-raw broken test-amd64-i386-xl-qemuu-debianhvm-amd64broken test-amd64-amd64-xl-qemuu-debianhvm-i386-xsmbroken test-amd64-amd64-xl-qemut-win7-amd64 broken test-amd64-i386-xl-qemuu-win7-amd64 4 host-install(4) broken REGR. vs. 144631 test-amd64-i386-xl-qemut-debianhvm-amd64 4 host-install(4) broken REGR. vs. 144631 test-amd64-i386-xl-qemuu-debianhvm-amd64 4 host-install(4) broken REGR. vs. 144631 test-amd64-i386-xl-qemut-debianhvm-i386-xsm broken in 144641 test-amd64-amd64-xl-qcow2broken in 144641 test-amd64-i386-xl-xsm broken in 144641 Tests which are failing intermittently (not blocking): test-amd64-i386-xl-xsm 4 host-install(4) broken in 144641 pass in 144647 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 4 host-install(4) broken in 144641 pass in 144647 test-amd64-i386-xl-raw4 host-install(4) broken pass in 144641 test-amd64-i386-freebsd10-amd64 4 host-install(4) broken pass in 144641 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 4 host-install(4) broken pass in 144641 test-amd64-i386-qemuu-rhel6hvm-intel 12 guest-start/redhat.repeat fail in 144641 pass in 144647 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 144641 pass in 144647 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail pass in 144641 test-amd64-i386-libvirt-pair 26 leak-check/check/src_host fail pass in 144641 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 17 guest-saverestore.2 fail REGR. vs. 144635 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qcow2 4 host-install(4) broken in 144641 like 144635 test-amd64-amd64-xl-qemut-win7-amd64 4 host-install(4) broken like 144635 test-amd64-amd64-xl-rtds 16 guest-localmigrate fail in 144641 like 144619 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 144631 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 144631 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144635 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 144635 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 144635 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144635 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 144635 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass
[Xen-devel] [PATCH] xen-blkback: prevent premature module unload
Objects allocated by xen_blkif_alloc come from the 'blkif_cache' kmem cache. This cache is destoyed when xen-blkif is unloaded so it is necessary to wait for the deferred free routine used for such objects to complete. This necessity was missed in commit 14855954f636 "xen-blkback: allow module to be cleanly unloaded". This patch fixes the problem by taking/releasing extra module references in xen_blkif_alloc/free() respectively. Signed-off-by: Paul Durrant --- Cc: Konrad Rzeszutek Wilk Cc: "Roger Pau Monné" Cc: Jens Axboe --- drivers/block/xen-blkback/xenbus.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c index e8c5c54e1d26..59d576d27ca7 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -171,6 +171,15 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid) blkif->domid = domid; atomic_set(>refcnt, 1); init_completion(>drain_complete); + + /* +* Because freeing back to the cache may be deferred, it is not +* safe to unload the module (and hence destroy the cache) until +* this has completed. To prevent premature unloading, take an +* extra module reference here and release only when the object +* has been free back to the cache. +*/ + __module_get(THIS_MODULE); INIT_WORK(>free_work, xen_blkif_deferred_free); return blkif; @@ -320,6 +329,7 @@ static void xen_blkif_free(struct xen_blkif *blkif) /* Make sure everything is drained before shutting down */ kmem_cache_free(xen_blkif_cachep, blkif); + module_put(THIS_MODULE); } int __init xen_blkif_interface_init(void) -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 144659: tolerable all pass - PUSHED
flight 144659 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/144659/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 4935a5433db28077fe6313f920bbedcd54516cec baseline version: xen b73aad4c8b6a767ce15cc8cb65f9eeab7bfccdae Last test of basis 144639 2019-12-09 14:00:23 Z1 days Testing same since 144659 2019-12-10 11:00:42 Z0 days1 attempts People who touched revisions under test: Igor Druzhinin Razvan Cojocaru 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 b73aad4c8b..4935a5433d 4935a5433db28077fe6313f920bbedcd54516cec -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/AMD: unbreak CPU hotplug on AMD systems without RstrFpErrPtrs
On 10/12/2019 13:58, Jan Beulich wrote: > On 10.12.2019 13:37, Andrew Cooper wrote: >> Furthermore, you will observe that there is an action item on me from >> the call to come up with a less broken alternative which I'm genuinely >> attempting to do. > > There's no indication towards this in the minutes, afaics. Or wait > - the same topic appears twice there (as both 4 and 6). I wasn't > even aware of such an action item. I'll be happy to revert if you > indicate so, and if a better fix is going to show up in time. I don't think reverting would make sense - the patch doesn't break anything, even more - it actually fixes the problem we observed. If there is an improvement to that coming - it should be just done on top of this also potentially taking care of other places in the code that might be affected. Igor ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [seabios test] 144649: trouble: broken/fail/pass
flight 144649 seabios real [real] http://logs.test-lab.xenproject.org/osstest/logs/144649/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-debianhvm-amd64 broken test-amd64-i386-xl-qemuu-debianhvm-i386-xsm broken in 144644 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow broken in 144644 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow broken in 144644 Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 4 host-install(4) broken in 144644 pass in 144649 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 4 host-install(4) broken in 144644 pass in 144649 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 4 host-install(4) broken in 144644 pass in 144649 test-amd64-amd64-xl-qemuu-debianhvm-amd64 4 host-install(4) broken pass in 144644 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144198 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144198 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144198 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 144198 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass version targeted for testing: seabios f21b5a4aeb020f2a5e2c6503f906a9349dd2f069 baseline version: seabios c9ba5276e3217ac6a1ec772dbebf568ba3a8a55d Last test of basis 144198 2019-11-18 14:08:47 Z 21 days Testing same since 144644 2019-12-09 21:08:58 Z0 days2 attempts People who touched revisions under test: Kevin O'Connor 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-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm pass test-amd64-i386-xl-qemuu-debianhvm-i386-xsm pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64broken test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-qemuu-ws16-amd64 fail test-amd64-i386-xl-qemuu-ws16-amd64 fail test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrictpass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict pass test-amd64-amd64-qemuu-nested-intel pass test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow pass test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 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 broken-job test-amd64-amd64-xl-qemuu-debianhvm-amd64 broken broken-step test-amd64-amd64-xl-qemuu-debianhvm-amd64 host-install(4) broken-job test-amd64-i386-xl-qemuu-debianhvm-i386-xsm broken broken-job test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow broken broken-job test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow broken Not pushing. commit f21b5a4aeb020f2a5e2c6503f906a9349dd2f069 Author: Kevin O'Connor Date: Mon Dec 9 15:08:17 2019 -0500 docs: Note v1.13.0 release Signed-off-by: Kevin
Re: [Xen-devel] [PATCH for-4.13 v4 1/3] xen/flask: Drop the gen-policy.py script
On 10.12.2019 13:17, Andrew Cooper wrote: > The script is Python 2 specific, and fails with string/binary issues with > Python 3: > > Traceback (most recent call last): > File "gen-policy.py", line 14, in > for char in sys.stdin.read(): > File "/usr/lib/python3.5/codecs.py", line 321, in decode > (result, consumed) = self._buffer_decode(data, self.errors, final) > UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: > invalid start byte > > Fixing the script to be compatible isn't hard, but using python here is > wasteful. Drop the script entirely, and write an equivelent flask-policy.S > instead. This removes the need for a $(PYTHON) and $(CC) pass. > > Signed-off-by: Andrew Cooper > Acked-by: Daniel De Graaf Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/AMD: unbreak CPU hotplug on AMD systems without RstrFpErrPtrs
On 10.12.2019 13:37, Andrew Cooper wrote:
> On 10/12/2019 10:26, Jürgen Groß wrote:
>> On 10.12.19 11:10, Jan Beulich wrote:
>>> On 04.12.2019 00:56, Igor Druzhinin wrote:
If the feature is not present Xen will try to force X86_BUG_FPU_PTRS
feature at CPU identification time. This is especially noticeable in
PV-shim that usually hotplugs its vCPUs. We either need to restrict
this
action for boot CPU only or allow secondary CPUs to modify
forced CPU capabilities at runtime. Choose the former since modifying
forced capabilities out of boot path leaves the system in potentially
inconsistent state.
Signed-off-by: Igor Druzhinin
>>>
>>> I've committed this to unstable, as per the outcome of the
>>> community call.
>
> What outcome? Yes technically your R-by is sufficient to get the patch
> in, but you know very well there are open objections against this
> version of the patch.
My proposal on the call was to go with the existing patch, and improve
from there. There wasn't great enthusiasm, but there was agreement that
this is the most pragmatic route to follow. In particular I don't
recall you voicing any objection to this on the call (I do very well
recall the objection you voiced earlier on, which I had responded to
with a suggestion of a slightly different approach, taking care [I
think] of your wishes as well as mine).
> Also, you're actually in a position where you are reviewing your own
> work, which is not how R-by is intended to work.
Which own work? The patch doesn't even carry a Suggested-by. Iirc
Igor told me that what has gone in is how he had it initially. So
I'm pretty confused by this statement of yours.
Furthermore, as a recurring pattern, simply not responding to ongoing
discussions should not, in the common case, lead to no progress at
all. Iirc this was also mentioned on the call ("lazy consensus").
> Furthermore, you will observe that there is an action item on me from
> the call to come up with a less broken alternative which I'm genuinely
> attempting to do.
There's no indication towards this in the minutes, afaics. Or wait
- the same topic appears twice there (as both 4 and 6). I wasn't
even aware of such an action item. I'll be happy to revert if you
indicate so, and if a better fix is going to show up in time.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] CODING_STYLE: Document how to handle unexpected conditions
On 10.12.2019 11:56, George Dunlap wrote: > On 12/9/19 1:50 PM, Jan Beulich wrote: >> On 09.12.2019 12:29, George Dunlap wrote: >>> --- a/CODING_STYLE >>> +++ b/CODING_STYLE >>> @@ -133,3 +133,97 @@ the end of files. It should be: >>> * indent-tabs-mode: nil >>> * End: >>> */ >>> + >>> +Handling unexpected conditions >>> +-- >>> + >>> +GUIDELINES: >>> + >>> +Passing errors up the stack should be used when the caller is already >>> +expecting to handle errors, and the state when the error was >>> +discovered isn’t broken, or isn't too hard to fix. >>> + >>> +domain_crash() should be used when passing errors up the stack is too >>> +difficult, and/or when fixing up state of a guest is impractical, but >>> +where fixing up the state of Xen will allow Xen to continue running. >>> +This is particularly appropriate when the guest is exhibiting behavior >>> +well-behaved guest should. >> >> DYM "shouldn't"? > > Indeed. (Btw, noticing only now - there's also either an "a" missing, or it wants to be "guests".) >>> +- domain_crash() is similar to BUG_ON(), but with a more limited >>> +effect: it stops that domain immediately. In situations where >>> +continuing might cause guest or hypervisor corruption, but destroying >>> +the guest allows the hypervisor to continue, this can change a more >>> +serious bug into a guest denial-of-service. But in situations where >>> +returning an error might be safe, then domain_crash() can change a >>> +benign failure into a guest denial-of-service. >> >> Perhaps further put emphasis on the call tree still getting unwound >> normally, which may imply further actions on the (now dying) domain >> taken. Unfortunately it's not unusual for people to forget this; I >> think the IOMMU code in particular was (hopefully isn't so much >> anymore) a "good" example of this. > > Can you expand on this? Do you mean to advise that care should be taken > when returning up the callstack that the domain which was running before > may now be dying, and to behave appropriately? One issue is with functions returning void, where the caller won't even know that something may have gone wrong. Another is that typically error paths are less commonly used, and crashing a domain would typically be accompanied by indicating an error to the upper layers. Hence such crashing may trigger unrelated bugs. A third aspect is that, indeed, dying guests may need special treatment (see the already existing ->is_dying checks we have). I mentioned the call tree unwinding in particular because earlier on we had domain_crash_synchronous(), which was there specifically to avoid issues with errors (and the changed state) bubbling back up. But this model had other issues, hence our movement away from it. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/3] xen/build: Automatically locate a suitable python interpreter
On 07.12.19 22:16, Andrew Cooper wrote: Needing to pass PYTHON=python3 into hypervisor builds is irritating and unnecessary. Locate a suitable interpreter automatically, defaulting to Py3 if it is available. Reported-by: Steven Haigh Signed-off-by: Andrew Cooper Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13 v4 1/3] xen/flask: Drop the gen-policy.py script
On 10.12.19 13:17, Andrew Cooper wrote: The script is Python 2 specific, and fails with string/binary issues with Python 3: Traceback (most recent call last): File "gen-policy.py", line 14, in for char in sys.stdin.read(): File "/usr/lib/python3.5/codecs.py", line 321, in decode (result, consumed) = self._buffer_decode(data, self.errors, final) UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0: invalid start byte Fixing the script to be compatible isn't hard, but using python here is wasteful. Drop the script entirely, and write an equivelent flask-policy.S instead. This removes the need for a $(PYTHON) and $(CC) pass. Signed-off-by: Andrew Cooper Acked-by: Daniel De Graaf Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/AMD: unbreak CPU hotplug on AMD systems without RstrFpErrPtrs
On 10/12/2019 10:26, Jürgen Groß wrote: > On 10.12.19 11:10, Jan Beulich wrote: >> On 04.12.2019 00:56, Igor Druzhinin wrote: >>> If the feature is not present Xen will try to force X86_BUG_FPU_PTRS >>> feature at CPU identification time. This is especially noticeable in >>> PV-shim that usually hotplugs its vCPUs. We either need to restrict >>> this >>> action for boot CPU only or allow secondary CPUs to modify >>> forced CPU capabilities at runtime. Choose the former since modifying >>> forced capabilities out of boot path leaves the system in potentially >>> inconsistent state. >>> >>> Signed-off-by: Igor Druzhinin >> >> I've committed this to unstable, as per the outcome of the >> community call. What outcome? Yes technically your R-by is sufficient to get the patch in, but you know very well there are open objections against this version of the patch. Also, you're actually in a position where you are reviewing your own work, which is not how R-by is intended to work. Furthermore, you will observe that there is an action item on me from the call to come up with a less broken alternative which I'm genuinely attempting to do. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.13 v4 1/3] xen/flask: Drop the gen-policy.py script
The script is Python 2 specific, and fails with string/binary issues with
Python 3:
Traceback (most recent call last):
File "gen-policy.py", line 14, in
for char in sys.stdin.read():
File "/usr/lib/python3.5/codecs.py", line 321, in decode
(result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x8c in position 0:
invalid start byte
Fixing the script to be compatible isn't hard, but using python here is
wasteful. Drop the script entirely, and write an equivelent flask-policy.S
instead. This removes the need for a $(PYTHON) and $(CC) pass.
Signed-off-by: Andrew Cooper
Acked-by: Daniel De Graaf
---
CC: Daniel De Graaf
CC: Juergen Gross
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: Julien Grall
CC: Volodymyr Babchuk
v2:
* Fix tabs vs spaces issues
v3:
* Use % rather than @ for progbits/object, for Arm32 build.
* Spotted by https://travis-ci.org/andyhhp/xen/builds/622085138
v4:
* Introduce and use ASM_INT() to declare something compatible with `int`
* Drop .align
For 4.13. This is a blocker to our intent to by Py3-clean in this release.
Discovered entirely accidently when testing the final patch.
---
xen/include/asm-arm/asm_defns.h | 6 ++
xen/include/asm-x86/asm_defns.h | 6 ++
xen/xsm/flask/Makefile | 6 ++
xen/xsm/flask/flask-policy.S| 16
xen/xsm/flask/gen-policy.py | 23 ---
5 files changed, 30 insertions(+), 27 deletions(-)
create mode 100644 xen/xsm/flask/flask-policy.S
delete mode 100644 xen/xsm/flask/gen-policy.py
diff --git a/xen/include/asm-arm/asm_defns.h b/xen/include/asm-arm/asm_defns.h
index 3f21def0ab..b4fbcdae1d 100644
--- a/xen/include/asm-arm/asm_defns.h
+++ b/xen/include/asm-arm/asm_defns.h
@@ -21,6 +21,12 @@
label: .asciz msg; \
.popsection
+#define ASM_INT(label, val) \
+.p2align 2; \
+label: .long (val); \
+.size label, . - label; \
+.type label, %object
+
#endif /* __ARM_ASM_DEFNS_H__ */
/*
* Local variables:
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index c4f49a35d3..370f239c50 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -386,4 +386,10 @@ static always_inline void stac(void)
4: .p2align 2; \
.popsection
+#define ASM_INT(label, val) \
+.p2align 2; \
+label: .long (val); \
+.size label, . - label; \
+.type label, @object
+
#endif /* __X86_ASM_DEFNS_H__ */
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index f5ffab1226..7c3f381287 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,7 +27,8 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
$(AV_H_FILES): $(AV_H_DEPEND)
$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)
-obj-$(CONFIG_XSM_FLASK_POLICY) += policy.o
+obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
+flask-policy.o: policy.bin
FLASK_BUILD_DIR := $(CURDIR)
POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
@@ -36,9 +37,6 @@ policy.bin: FORCE
$(MAKE) -f $(XEN_ROOT)/tools/flask/policy/Makefile.common -C
$(XEN_ROOT)/tools/flask/policy FLASK_BUILD_DIR=$(FLASK_BUILD_DIR)
cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
-policy.c: policy.bin gen-policy.py
- $(PYTHON) gen-policy.py < $< > $@
-
.PHONY: clean
clean::
rm -f $(ALL_H_FILES) *.o $(DEPS_RM) policy.* $(POLICY_SRC)
diff --git a/xen/xsm/flask/flask-policy.S b/xen/xsm/flask/flask-policy.S
new file mode 100644
index 00..e9308aa175
--- /dev/null
+++ b/xen/xsm/flask/flask-policy.S
@@ -0,0 +1,16 @@
+#include
+
+.section .init.rodata, "a", %progbits
+
+/* const unsigned char xsm_flask_init_policy[] __initconst */
+.global xsm_flask_init_policy
+xsm_flask_init_policy:
+.incbin "policy.bin"
+.Lend:
+
+.type xsm_flask_init_policy, %object
+.size xsm_flask_init_policy, . - xsm_flask_init_policy
+
+/* const unsigned int __initconst xsm_flask_init_policy_size */
+ .global xsm_flask_init_policy_size
+ASM_INT(xsm_flask_init_policy_size, .Lend - xsm_flask_init_policy)
diff --git a/xen/xsm/flask/gen-policy.py b/xen/xsm/flask/gen-policy.py
deleted file mode 100644
index c7501e4614..00
--- a/xen/xsm/flask/gen-policy.py
+++ /dev/null
@@ -1,23 +0,0 @@
-#!/usr/bin/env python
-import sys
-
-policy_size = 0
-
-sys.stdout.write("""
-/* This file is autogenerated by gen_policy.py */
-#include
-#include
-
-const unsigned char xsm_flask_init_policy[] __initconst = {
-""")
-
-for char in sys.stdin.read():
-sys.stdout.write(" 0x%02x," % ord(char))
-policy_size = policy_size + 1
-if policy_size % 13 == 0:
-
[Xen-devel] [ovmf test] 144651: regressions - FAIL
flight 144651 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/144651/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 144637 build-i386-xsm6 xen-buildfail REGR. vs. 144637 build-amd64-xsm 6 xen-buildfail REGR. vs. 144637 build-i3866 xen-buildfail REGR. vs. 144637 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a build-i386-libvirt1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a version targeted for testing: ovmf a80032dc44a1071a34f4415a7c5cef5170ee6159 baseline version: ovmf 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0 Last test of basis 144637 2019-12-09 09:09:49 Z1 days Failing since144646 2019-12-10 01:39:53 Z0 days2 attempts Testing same since 144651 2019-12-10 06:52:32 Z0 days1 attempts People who touched revisions under test: Bob Feng Steven Shi jobs: build-amd64-xsm fail build-i386-xsm fail build-amd64 fail build-i386 fail build-amd64-libvirt blocked build-i386-libvirt blocked build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 blocked test-amd64-i386-xl-qemuu-ovmf-amd64 blocked 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 Not pushing. commit a80032dc44a1071a34f4415a7c5cef5170ee6159 Author: Steven Shi Date: Tue Nov 19 16:22:08 2019 +0800 BaseTools: Remove redundant binary cache file Redesign the binary cache and not need to save the cache intermediate result and state in memory as a ModuleBuildCacheIR class instance. So remove the CacheIR.py which define the ModuleBuildCacheIR class. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit fc8b8deac2d77524ff8cfe44acf95b5e1f59804e Author: Steven Shi Date: Tue Nov 19 16:17:00 2019 +0800 BaseTools: Leverage compiler output to optimize binary cache Redesign the binary cache and bases on the compiler to output the dependency header files info for every module. The binary cache will directly consume the dependency header files info and doesn't parse the C source code by iteself. Also redesign the dependency files list format for module and try to share the common lib hash result as more as possible in local process. Remove the unnecessary share data access across multiprocessing. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit 3bfbc915074a45f4d9c61aa2b698a62f1a24124e Author: Steven Shi Date: Mon Oct 21 14:51:49 2019 +0800 BaseTools: enhance the CacheCopyFile method arg names Enhance the CacheCopyFile method arg names to be more clear and readable Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit 91f6c533f8e9c49ffd098e9167724596ecfd7410 Author: Steven Shi Date: Mon Oct 21 14:24:57 2019 +0800 BaseTools: store more complete output files in binary cache Binary cache use the OutputFile method to return the module built output files needed to store in cache, but current OutputFile implementation doesn't return complete output files. Enhance the OutputFile method to return more complete output files. Signed-off-by: Steven Shi Cc: Liming Gao Cc: Bob Feng Reviewed-by: Bob Feng commit 0c3e8e9947a6c13b4327dd11b20acb95441701cf Author: Bob Feng Date: Wed Nov 20
Re: [Xen-devel] [PATCH 3/4] xen/interface: don't discard pending work in FRONT/BACK_RING_ATTACH
On 09.12.19 17:38, Durrant, Paul wrote:
-Original Message-
From: Jürgen Groß
Sent: 09 December 2019 13:55
To: Durrant, Paul ; linux-ker...@vger.kernel.org;
xen-devel@lists.xenproject.org
Cc: Boris Ostrovsky ; Stefano Stabellini
Subject: Re: [PATCH 3/4] xen/interface: don't discard pending work in
FRONT/BACK_RING_ATTACH
On 05.12.19 15:01, Paul Durrant wrote:
Currently these macros will skip over any requests/responses that are
added to the shared ring whilst it is detached. This, in general, is not
a desirable semantic since most frontend implementations will eventually
block waiting for a response which would either never appear or never be
processed.
NOTE: These macros are currently unused. BACK_RING_ATTACH(), however,
will
be used in a subsequent patch.
Signed-off-by: Paul Durrant
---
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Stefano Stabellini
---
include/xen/interface/io/ring.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/xen/interface/io/ring.h
b/include/xen/interface/io/ring.h
index 3f40501fc60b..405adfed87e6 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -143,14 +143,14 @@ struct __name##_back_ring {
\
#define FRONT_RING_ATTACH(_r, _s, __size) do {
\
(_r)->sring = (_s); \
(_r)->req_prod_pvt = (_s)->req_prod; \
-(_r)->rsp_cons = (_s)->rsp_prod; \
+(_r)->rsp_cons = (_s)->req_prod; \
(_r)->nr_ents = __RING_SIZE(_s, __size); \
} while (0)
#define BACK_RING_ATTACH(_r, _s, __size) do {
\
(_r)->sring = (_s); \
(_r)->rsp_prod_pvt = (_s)->rsp_prod; \
-(_r)->req_cons = (_s)->req_prod; \
+(_r)->req_cons = (_s)->rsp_prod; \
(_r)->nr_ents = __RING_SIZE(_s, __size); \
} while (0)
Lets look at all possible scenarios where BACK_RING_ATTACH()
might happen:
Initially (after [FRONT|BACK]_RING_INIT(), leaving _pvt away):
req_prod=0, rsp_cons=0, rsp_prod=0, req_cons=0
Using BACK_RING_ATTACH() is fine (no change)
Request queued:
req_prod=1, rsp_cons=0, rsp_prod=0, req_cons=0
Using BACK_RING_ATTACH() is fine (no change)
and taken by backend:
req_prod=1, rsp_cons=0, rsp_prod=0, req_cons=1
Using BACK_RING_ATTACH() is resetting req_cons to 0, will result
in redoing request (for blk this is fine, other devices like SCSI
tapes will have issues with that). One possible solution would be
to ensure all taken requests are either stopped or the response
is queued already.
Yes, it is the assumption that a backend will drain and complete any requests
it is handling, but it will not deal with new ones being posted by the
frontend. This does appear to be the case for blkback.
Response queued:
req_prod=1, rsp_cons=0, rsp_prod=1, req_cons=1
Using BACK_RING_ATTACH() is fine (no change)
Response taken:
req_prod=1, rsp_cons=1, rsp_prod=1, req_cons=1
Using BACK_RING_ATTACH() is fine (no change)
In general I believe the [FRONT|BACK]_RING_ATTACH() macros are not
fine to be used in the current state, as the *_pvt fields normally not
accessible by the other end are initialized using the (possibly
untrusted) values from the shared ring. There needs at least to be a
test for the values to be sane, and your change should not result in the
same value to be read twice, as it could have changed in between.
What test would you apply to sanitize the value of the pvt pointer?
For the BACK_RING_ATTACH() case rsp_prod_pvt should not be between
req_prod and req_cons, and req_cons - rsp_prod_pvt should be <= ring
size IMO.
Another option would be to have a backend write its pvt value into the xenstore
backend area when the ring is unmapped, so that a new instance definitely
resumes where the old one left off. The value of rsp_prod could, of course, be
overwritten by the guest at any time and so there's little point in attempting
sanitize it.
I don't think this would be necessary. With above validation in place
all the guest could do would be to shoot itself in the foot.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 2/4] xenbus: limit when state is forced to closed
If a driver probe() fails then leave the xenstore state alone. There is no
reason to modify it as the failure may be due to transient resource
allocation issues and hence a subsequent probe() may succeed.
If the driver supports re-binding then only force state to closed during
remove() only in the case when the toolstack may need to clean up. This can
be detected by checking whether the state in xenstore has been set to
closing prior to device removal.
NOTE: Re-bind support is indicated by new boolean in struct xenbus_driver,
which defaults to false. Subsequent patches will add support to
some backend drivers.
Signed-off-by: Paul Durrant
---
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Stefano Stabellini
v2:
- Introduce the 'allow_rebind' flag
- Expand the commit comment
---
drivers/xen/xenbus/xenbus_probe.c | 12 ++--
include/xen/xenbus.h | 1 +
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_probe.c
b/drivers/xen/xenbus/xenbus_probe.c
index a10311c348b9..9303ff35b2bd 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -255,7 +255,6 @@ int xenbus_dev_probe(struct device *_dev)
module_put(drv->driver.owner);
fail:
xenbus_dev_error(dev, err, "xenbus_dev_probe on %s", dev->nodename);
- xenbus_switch_state(dev, XenbusStateClosed);
return err;
}
EXPORT_SYMBOL_GPL(xenbus_dev_probe);
@@ -276,7 +275,16 @@ int xenbus_dev_remove(struct device *_dev)
free_otherend_details(dev);
- xenbus_switch_state(dev, XenbusStateClosed);
+ /*
+* If the toolstack has forced the device state to closing then set
+* the state to closed now to allow it to be cleaned up.
+* Similarly, if the driver does not support re-bind, set the
+* closed.
+*/
+ if (!drv->allow_rebind ||
+ xenbus_read_driver_state(dev->nodename) == XenbusStateClosing)
+ xenbus_switch_state(dev, XenbusStateClosed);
+
return 0;
}
EXPORT_SYMBOL_GPL(xenbus_dev_remove);
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 869c816d5f8c..24228a102141 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -93,6 +93,7 @@ struct xenbus_device_id
struct xenbus_driver {
const char *name; /* defaults to ids[0].devicetype */
const struct xenbus_device_id *ids;
+ bool allow_rebind; /* avoid setting xenstore closed during remove */
int (*probe)(struct xenbus_device *dev,
const struct xenbus_device_id *id);
void (*otherend_changed)(struct xenbus_device *dev,
--
2.20.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 3/4] xen/interface: re-define FRONT/BACK_RING_ATTACH()
Currently these macros are defined to re-initialize a front/back ring
(respectively) to values read from the shared ring in such a way that any
requests/responses that are added to the shared ring whilst the front/back
is detached will be skipped over. This, in general, is not a desirable
semantic since most frontend implementations will eventually block waiting
for a response which would either never appear or never be processed.
Since the macros are currently unused, take this opportunity to re-define
them to re-initialize a front/back ring using specified values. This also
allows FRONT/BACK_RING_INIT() to be re-defined in terms of
FRONT/BACK_RING_ATTACH() using a specified value of 0.
NOTE: BACK_RING_ATTACH() will be used directly in a subsequent patch.
Signed-off-by: Paul Durrant
---
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Stefano Stabellini
A patch to add the FRONT/BACK_RING_ATTACH() macros to the canonical
ring.h in xen.git will be sent once the definitions have been agreed.
v2:
- change definitions to take explicit initial index values
---
include/xen/interface/io/ring.h | 29 +
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index 3f40501fc60b..2af7a1cd6658 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -125,35 +125,24 @@ struct __name##_back_ring {
\
memset((_s)->pad, 0, sizeof((_s)->pad)); \
} while(0)
-#define FRONT_RING_INIT(_r, _s, __size) do { \
-(_r)->req_prod_pvt = 0;\
-(_r)->rsp_cons = 0;
\
+#define FRONT_RING_ATTACH(_r, _s, _i, __size) do { \
+(_r)->req_prod_pvt = (_i); \
+(_r)->rsp_cons = (_i); \
(_r)->nr_ents = __RING_SIZE(_s, __size); \
(_r)->sring = (_s);
\
} while (0)
-#define BACK_RING_INIT(_r, _s, __size) do {\
-(_r)->rsp_prod_pvt = 0;\
-(_r)->req_cons = 0;
\
-(_r)->nr_ents = __RING_SIZE(_s, __size); \
-(_r)->sring = (_s);
\
-} while (0)
+#define FRONT_RING_INIT(_r, _s, __size) FRONT_RING_ATTACH(_r, _s, 0, __size)
-/* Initialize to existing shared indexes -- for recovery */
-#define FRONT_RING_ATTACH(_r, _s, __size) do { \
-(_r)->sring = (_s);
\
-(_r)->req_prod_pvt = (_s)->req_prod; \
-(_r)->rsp_cons = (_s)->rsp_prod; \
+#define BACK_RING_ATTACH(_r, _s, _i, __size) do { \
+(_r)->rsp_prod_pvt = (_i); \
+(_r)->req_cons = (_i); \
(_r)->nr_ents = __RING_SIZE(_s, __size); \
-} while (0)
-
-#define BACK_RING_ATTACH(_r, _s, __size) do { \
(_r)->sring = (_s);
\
-(_r)->rsp_prod_pvt = (_s)->rsp_prod; \
-(_r)->req_cons = (_s)->req_prod; \
-(_r)->nr_ents = __RING_SIZE(_s, __size); \
} while (0)
+#define BACK_RING_INIT(_r, _s, __size) BACK_RING_ATTACH(_r, _s, 0, __size)
+
/* How big is this ring? */
#define RING_SIZE(_r) \
((_r)->nr_ents)
--
2.20.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 1/4] xenbus: move xenbus_dev_shutdown() into frontend code...
...and make it static
xenbus_dev_shutdown() is seemingly intended to cause clean shutdown of PV
frontends when a guest is rebooted. Indeed the function waits for a
conpletion which is only set by a call to xenbus_frontend_closed().
This patch removes the shutdown() method from backends and moves
xenbus_dev_shutdown() from xenbus_probe.c into xenbus_probe_frontend.c,
renaming it appropriately and making it static.
NOTE: In the case where the backend is running in a driver domain, the
toolstack should have already terminated any frontends that may be
using it (since Xen does not support re-startable PV driver domains)
so xenbus_dev_shutdown() should never be called.
Signed-off-by: Paul Durrant
Reviewed-by: Juergen Gross
---
Cc: Boris Ostrovsky
Cc: Stefano Stabellini
---
drivers/xen/xenbus/xenbus.h| 2 --
drivers/xen/xenbus/xenbus_probe.c | 23 -
drivers/xen/xenbus/xenbus_probe_backend.c | 1 -
drivers/xen/xenbus/xenbus_probe_frontend.c | 24 +-
4 files changed, 23 insertions(+), 27 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus.h b/drivers/xen/xenbus/xenbus.h
index d75a2385b37c..5f5b8a7d5b80 100644
--- a/drivers/xen/xenbus/xenbus.h
+++ b/drivers/xen/xenbus/xenbus.h
@@ -116,8 +116,6 @@ int xenbus_probe_devices(struct xen_bus_type *bus);
void xenbus_dev_changed(const char *node, struct xen_bus_type *bus);
-void xenbus_dev_shutdown(struct device *_dev);
-
int xenbus_dev_suspend(struct device *dev);
int xenbus_dev_resume(struct device *dev);
int xenbus_dev_cancel(struct device *dev);
diff --git a/drivers/xen/xenbus/xenbus_probe.c
b/drivers/xen/xenbus/xenbus_probe.c
index 4461f4583476..a10311c348b9 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -281,29 +281,6 @@ int xenbus_dev_remove(struct device *_dev)
}
EXPORT_SYMBOL_GPL(xenbus_dev_remove);
-void xenbus_dev_shutdown(struct device *_dev)
-{
- struct xenbus_device *dev = to_xenbus_device(_dev);
- unsigned long timeout = 5*HZ;
-
- DPRINTK("%s", dev->nodename);
-
- get_device(>dev);
- if (dev->state != XenbusStateConnected) {
- pr_info("%s: %s: %s != Connected, skipping\n",
- __func__, dev->nodename, xenbus_strstate(dev->state));
- goto out;
- }
- xenbus_switch_state(dev, XenbusStateClosing);
- timeout = wait_for_completion_timeout(>down, timeout);
- if (!timeout)
- pr_info("%s: %s timeout closing device\n",
- __func__, dev->nodename);
- out:
- put_device(>dev);
-}
-EXPORT_SYMBOL_GPL(xenbus_dev_shutdown);
-
int xenbus_register_driver_common(struct xenbus_driver *drv,
struct xen_bus_type *bus,
struct module *owner, const char *mod_name)
diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c
b/drivers/xen/xenbus/xenbus_probe_backend.c
index b0bed4faf44c..14876faff3b0 100644
--- a/drivers/xen/xenbus/xenbus_probe_backend.c
+++ b/drivers/xen/xenbus/xenbus_probe_backend.c
@@ -198,7 +198,6 @@ static struct xen_bus_type xenbus_backend = {
.uevent = xenbus_uevent_backend,
.probe = xenbus_dev_probe,
.remove = xenbus_dev_remove,
- .shutdown = xenbus_dev_shutdown,
.dev_groups = xenbus_dev_groups,
},
};
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c
b/drivers/xen/xenbus/xenbus_probe_frontend.c
index a7d90a719cea..8a1650bbe18f 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -126,6 +126,28 @@ static int xenbus_frontend_dev_probe(struct device *dev)
return xenbus_dev_probe(dev);
}
+static void xenbus_frontend_dev_shutdown(struct device *_dev)
+{
+ struct xenbus_device *dev = to_xenbus_device(_dev);
+ unsigned long timeout = 5*HZ;
+
+ DPRINTK("%s", dev->nodename);
+
+ get_device(>dev);
+ if (dev->state != XenbusStateConnected) {
+ pr_info("%s: %s: %s != Connected, skipping\n",
+ __func__, dev->nodename, xenbus_strstate(dev->state));
+ goto out;
+ }
+ xenbus_switch_state(dev, XenbusStateClosing);
+ timeout = wait_for_completion_timeout(>down, timeout);
+ if (!timeout)
+ pr_info("%s: %s timeout closing device\n",
+ __func__, dev->nodename);
+ out:
+ put_device(>dev);
+}
+
static const struct dev_pm_ops xenbus_pm_ops = {
.suspend= xenbus_dev_suspend,
.resume = xenbus_frontend_dev_resume,
@@ -146,7 +168,7 @@ static struct xen_bus_type xenbus_frontend = {
.uevent = xenbus_uevent_frontend,
.probe = xenbus_frontend_dev_probe,
.remove = xenbus_dev_remove,
-
[Xen-devel] [PATCH v2 0/4] xen-blkback: support live update
Patch #1 is clean-up for an apparent mis-feature. Paul Durrant (4): xenbus: move xenbus_dev_shutdown() into frontend code... xenbus: limit when state is forced to closed xen/interface: re-define FRONT/BACK_RING_ATTACH() xen-blkback: support dynamic unbind/bind drivers/block/xen-blkback/xenbus.c | 59 +++--- drivers/xen/xenbus/xenbus.h| 2 - drivers/xen/xenbus/xenbus_probe.c | 35 - drivers/xen/xenbus/xenbus_probe_backend.c | 1 - drivers/xen/xenbus/xenbus_probe_frontend.c | 24 - include/xen/interface/io/ring.h| 29 --- include/xen/xenbus.h | 1 + 7 files changed, 84 insertions(+), 67 deletions(-) --- Cc: Boris Ostrovsky Cc: Jens Axboe Cc: Juergen Gross Cc: Konrad Rzeszutek Wilk Cc: "Roger Pau Monné" Cc: Stefano Stabellini -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 4/4] xen-blkback: support dynamic unbind/bind
By simply re-attaching to shared rings during connect_ring() rather than
assuming they are freshly allocated (i.e assuming the counters are zero)
it is possible for vbd instances to be unbound and re-bound from and to
(respectively) a running guest.
This has been tested by running:
while true;
do fio --name=randwrite --ioengine=libaio --iodepth=16 \
--rw=randwrite --bs=4k --direct=1 --size=1G --verify=crc32;
done
in a PV guest whilst running:
while true;
do echo vbd-$DOMID-$VBD >unbind;
echo unbound;
sleep 5;
echo vbd-$DOMID-$VBD >bind;
echo bound;
sleep 3;
done
in dom0 from /sys/bus/xen-backend/drivers/vbd to continuously unbind and
re-bind its system disk image.
This is a highly useful feature for a backend module as it allows it to be
unloaded and re-loaded (i.e. updated) without requiring domUs to be halted.
This was also tested by running:
while true;
do echo vbd-$DOMID-$VBD >unbind;
echo unbound;
sleep 5;
rmmod xen-blkback;
echo unloaded;
sleep 1;
modprobe xen-blkback;
echo bound;
cd $(pwd);
sleep 3;
done
in dom0 whilst running the same loop as above in the (single) PV guest.
Some (less stressful) testing has also been done using a Windows HVM guest
with the latest 9.0 PV drivers installed.
Signed-off-by: Paul Durrant
---
Cc: Konrad Rzeszutek Wilk
Cc: "Roger Pau Monné"
Cc: Jens Axboe
Cc: Boris Ostrovsky
Cc: Juergen Gross
Cc: Stefano Stabellini
v2:
- Apply a sanity check to the value of rsp_prod and fail the re-attach
if it is implausible
- Set allow_rebind to prevent ring from being closed on unbind
- Update test workload from dd to fio (with verification)
---
drivers/block/xen-blkback/xenbus.c | 59 +-
1 file changed, 41 insertions(+), 18 deletions(-)
diff --git a/drivers/block/xen-blkback/xenbus.c
b/drivers/block/xen-blkback/xenbus.c
index e8c5c54e1d26..13d09630b237 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -181,6 +181,8 @@ static int xen_blkif_map(struct xen_blkif_ring *ring,
grant_ref_t *gref,
{
int err;
struct xen_blkif *blkif = ring->blkif;
+ struct blkif_common_sring *sring_common;
+ RING_IDX rsp_prod, req_prod;
/* Already connected through? */
if (ring->irq)
@@ -191,46 +193,66 @@ static int xen_blkif_map(struct xen_blkif_ring *ring,
grant_ref_t *gref,
if (err < 0)
return err;
+ sring_common = (struct blkif_common_sring *)ring->blk_ring;
+ rsp_prod = READ_ONCE(sring_common->rsp_prod);
+ req_prod = READ_ONCE(sring_common->req_prod);
+
switch (blkif->blk_protocol) {
case BLKIF_PROTOCOL_NATIVE:
{
- struct blkif_sring *sring;
- sring = (struct blkif_sring *)ring->blk_ring;
- BACK_RING_INIT(>blk_rings.native, sring,
- XEN_PAGE_SIZE * nr_grefs);
+ struct blkif_sring *sring_native =
+ (struct blkif_sring *)ring->blk_ring;
+ unsigned int size = __RING_SIZE(sring_native,
+ XEN_PAGE_SIZE * nr_grefs);
+
+ BACK_RING_ATTACH(>blk_rings.native, sring_native,
+rsp_prod, XEN_PAGE_SIZE * nr_grefs);
+ err = (req_prod - rsp_prod > size) ? -EIO : 0;
break;
}
case BLKIF_PROTOCOL_X86_32:
{
- struct blkif_x86_32_sring *sring_x86_32;
- sring_x86_32 = (struct blkif_x86_32_sring *)ring->blk_ring;
- BACK_RING_INIT(>blk_rings.x86_32, sring_x86_32,
- XEN_PAGE_SIZE * nr_grefs);
+ struct blkif_x86_32_sring *sring_x86_32 =
+ (struct blkif_x86_32_sring *)ring->blk_ring;
+ unsigned int size = __RING_SIZE(sring_x86_32,
+ XEN_PAGE_SIZE * nr_grefs);
+
+ BACK_RING_ATTACH(>blk_rings.x86_32, sring_x86_32,
+rsp_prod, XEN_PAGE_SIZE * nr_grefs);
+ err = (req_prod - rsp_prod > size) ? -EIO : 0;
break;
}
case BLKIF_PROTOCOL_X86_64:
{
- struct blkif_x86_64_sring *sring_x86_64;
- sring_x86_64 = (struct blkif_x86_64_sring *)ring->blk_ring;
- BACK_RING_INIT(>blk_rings.x86_64, sring_x86_64,
- XEN_PAGE_SIZE * nr_grefs);
+ struct blkif_x86_64_sring *sring_x86_64 =
+ (struct blkif_x86_64_sring *)ring->blk_ring;
+ unsigned int size = __RING_SIZE(sring_x86_64,
+ XEN_PAGE_SIZE * nr_grefs);
+
+ BACK_RING_ATTACH(>blk_rings.x86_64, sring_x86_64,
+rsp_prod, XEN_PAGE_SIZE * nr_grefs);
+ err = (req_prod - rsp_prod > size) ? -EIO : 0;
Re: [Xen-devel] [PATCH] xen/blkfront: Adjust indentation in xlvbd_alloc_gendisk
On Tue, Dec 10, 2019 at 08:15:22AM +0100, Jürgen Groß wrote:
> On 09.12.19 21:14, Nathan Chancellor wrote:
> > Clang warns:
> >
> > ../drivers/block/xen-blkfront.c:1117:4: warning: misleading indentation;
> > statement is not part of the previous 'if' [-Wmisleading-indentation]
> > nr_parts = PARTS_PER_DISK;
> > ^
> > ../drivers/block/xen-blkfront.c:1115:3: note: previous statement is here
> > if (err)
> > ^
> >
> > This is because there is a space at the beginning of this line; remove
> > it so that the indentation is consistent according to the Linux kernel
> > coding style and clang no longer warns.
> >
> > While we are here, the previous line has some trailing whitespace; clean
> > that up as well.
> >
> > Fixes: c80a420995e7 ("xen-blkfront: handle Xen major numbers other than
> > XENVBD")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/791
> > Signed-off-by: Nathan Chancellor
>
> Reviewed-by: Juergen Gross
Acked-by: Roger Pau Monné
Thanks.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 2/2] xen/blkback: Squeeze page pools if a memory pressure is detected
On Tue, Dec 10, 2019 at 08:06:28AM +, SeongJae Park wrote: > Each `blkif` has a free pages pool for the grant mapping. The size of > the pool starts from zero and be increased on demand while processing > the I/O requests. If current I/O requests handling is finished or 100 > milliseconds has passed since last I/O requests handling, it checks and > shrinks the pool to not exceed the size limit, `max_buffer_pages`. > > Therefore, `blkfront` running guests can cause a memory pressure in the > `blkback` running guest by attaching a large number of block devices and > inducing I/O. Hm, I don't think this is actually true. blkfront cannot attach an arbitrary number of devices, blkfront is just a frontend for a device that's instantiated by the Xen toolstack, so it's the toolstack the one that controls the amount of PV block devices. > System administrators can avoid such problematic > situations by limiting the maximum number of devices each guest can > attach. However, finding the optimal limit is not so easy. Improper > set of the limit can results in the memory pressure or a resource > underutilization. This commit avoids such problematic situations by > squeezing the pools (returns every free page in the pool to the system) > for a while (users can set this duration via a module parameter) if a > memory pressure is detected. > > Discussions > === > > The `blkback`'s original shrinking mechanism returns only pages in the > pool, which are not currently be used by `blkback`, to the system. In > other words, the pages are not mapped with foreign pages. Because this ^ that ^ granted > commit is changing only the shrink limit but uses the mechanism as is, > this commit does not introduce improper mappings related security > issues. That last sentence is hard to parse. I think something like: "Because this commit is changing only the shrink limit but still uses the same freeing mechanism it does not touch pages which are currently mapping grants." > > Once a memory pressure is detected, this commit keeps the squeezing > limit for a user-specified time duration. The duration should be > neither too long nor too short. If it is too long, the squeezing > incurring overhead can reduce the I/O performance. If it is too short, > `blkback` will not free enough pages to reduce the memory pressure. > This commit sets the value as `10 milliseconds` by default because it is > a short time in terms of I/O while it is a long time in terms of memory > operations. Also, as the original shrinking mechanism works for at > least every 100 milliseconds, this could be a somewhat reasonable > choice. I also tested other durations (refer to the below section for > more details) and confirmed that 10 milliseconds is the one that works > best with the test. That said, the proper duration depends on actual > configurations and workloads. That's why this commit is allowing users ^ allows > to set it as their optimal value via the module parameter. ... to set the duration as a module parameter. > > Memory Pressure Test > > > To show how this commit fixes the memory pressure situation well, I > configured a test environment on a xen-running virtualization system. > On the `blkfront` running guest instances, I attach a large number of > network-backed volume devices and induce I/O to those. Meanwhile, I > measure the number of pages that swapped in and out on the `blkback` > running guest. The test ran twice, once for the `blkback` before this > commit and once for that after this commit. As shown below, this commit > has dramatically reduced the memory pressure: > > pswpin pswpout I assume pswpin means 'pages swapped in' and pswpout 'pages swapped out'. Might be good to add a note to that effect. > before 76,672 185,799 > after 2123,325 > > Optimal Aggressive Shrinking Duration > - > > To find a best squeezing duration, I repeated the test with three > different durations (1ms, 10ms, and 100ms). The results are as below: > > durationpswpin pswpout > 1 852 6,424 > 10 212 3,325 > 100 203 3,340 > > As expected, the memory pressure has decreased as the duration is > increased, but the reduction stopped from the `10ms`. Based on this > results, I chose the default duration as 10ms. > > Performance Overhead Test > = > > This commit could incur I/O performance degradation under severe memory > pressure because the squeezing will require more page allocations per > I/O. To show the overhead, I artificially made a worst-case squeezing > situation and measured the I/O performance of a `blkfront` running > guest. > > For the artificial squeezing, I set the `blkback.max_buffer_pages` using > the
Re: [Xen-devel] [PATCH v3] CODING_STYLE: Document how to handle unexpected conditions
On 12/9/19 1:50 PM, Jan Beulich wrote: > On 09.12.2019 12:29, George Dunlap wrote: >> --- a/CODING_STYLE >> +++ b/CODING_STYLE >> @@ -133,3 +133,97 @@ the end of files. It should be: >> * indent-tabs-mode: nil >> * End: >> */ >> + >> +Handling unexpected conditions >> +-- >> + >> +GUIDELINES: >> + >> +Passing errors up the stack should be used when the caller is already >> +expecting to handle errors, and the state when the error was >> +discovered isn’t broken, or isn't too hard to fix. >> + >> +domain_crash() should be used when passing errors up the stack is too >> +difficult, and/or when fixing up state of a guest is impractical, but >> +where fixing up the state of Xen will allow Xen to continue running. >> +This is particularly appropriate when the guest is exhibiting behavior >> +well-behaved guest should. > > DYM "shouldn't"? Indeed. >> +- domain_crash() is similar to BUG_ON(), but with a more limited >> +effect: it stops that domain immediately. In situations where >> +continuing might cause guest or hypervisor corruption, but destroying >> +the guest allows the hypervisor to continue, this can change a more >> +serious bug into a guest denial-of-service. But in situations where >> +returning an error might be safe, then domain_crash() can change a >> +benign failure into a guest denial-of-service. > > Perhaps further put emphasis on the call tree still getting unwound > normally, which may imply further actions on the (now dying) domain > taken. Unfortunately it's not unusual for people to forget this; I > think the IOMMU code in particular was (hopefully isn't so much > anymore) a "good" example of this. Can you expand on this? Do you mean to advise that care should be taken when returning up the callstack that the domain which was running before may now be dying, and to behave appropriately? -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] x86: do not enable global pages when virtualized on AMD hardware
On 10.12.2019 11:18, Roger Pau Monné wrote:
> On Tue, Dec 10, 2019 at 11:11:18AM +0100, Jan Beulich wrote:
>> On 09.12.2019 18:37, Roger Pau Monne wrote:
>>> --- a/xen/arch/x86/pv/domain.c
>>> +++ b/xen/arch/x86/pv/domain.c
>>> @@ -118,6 +118,19 @@ unsigned long pv_fixup_guest_cr4(const struct vcpu *v,
>>> unsigned long cr4)
>>> (mmu_cr4_features & PV_CR4_GUEST_VISIBLE_MASK));
>>> }
>>>
>>> +static int8_t __read_mostly opt_global_pages = -1;
>>> +boolean_runtime_param("global-pages", opt_global_pages);
>>> +
>>> +static int __init pge_init(void)
>>> +{
>>> +if ( opt_global_pages == -1 )
>>> +opt_global_pages = !cpu_has_hypervisor ||
>>> + boot_cpu_data.x86_vendor != X86_VENDOR_AMD;
>>
>> I was about to commit this when I noticed - what about Hygon here?
>
> Oh the vendor ID is different albeit it's just a clone. Please feel
> free to add it at commit.
>
> I also wonder: it might be good to have some kind of macro that
> matches both AMD and Hygon (IS_AMD_COMPAT or some such) in order to
> avoid this kind of mistakes in the future.
Because it's a clone, down the road this may be more risky. Here
what we're really interested in is SVM, just that we can't check
the feature flag (because it may not be exposed to us).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/6] x86/smt: Don't use -EBUSY for smt_up_down_helper() continuations
On 05.12.2019 23:30, Andrew Cooper wrote:
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -85,6 +85,9 @@ long cpu_up_helper(void *data)
> /* On EBUSY, flush RCU work and have one more go. */
> rcu_barrier();
> ret = cpu_up(cpu);
> +
> +if ( ret == -EBUSY )
> +ret = -ERESTART;
> }
>
> if ( !ret && !opt_smt &&
> @@ -110,6 +113,9 @@ long cpu_down_helper(void *data)
> /* On EBUSY, flush RCU work and have one more go. */
> rcu_barrier();
> ret = cpu_down(cpu);
> +
> +if ( ret == -EBUSY )
> +ret = -ERESTART;
> }
> return ret;
> }
For both of these - if two successive attempts didn't work, is
there really much point not reporting the fact back to the
caller? You're liable to request continuations indefinitely then.
> @@ -143,8 +149,7 @@ static long smt_up_down_helper(void *data)
> */
> if ( ret != -EEXIST && general_preempt_check() )
> {
> -/* In tasklet context - can't create a contination. */
> -ret = -EBUSY;
> +ret = -ERESTART;
> break;
> }
>
I agree with this change.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/AMD: unbreak CPU hotplug on AMD systems without RstrFpErrPtrs
On 10.12.19 11:10, Jan Beulich wrote: On 04.12.2019 00:56, Igor Druzhinin wrote: If the feature is not present Xen will try to force X86_BUG_FPU_PTRS feature at CPU identification time. This is especially noticeable in PV-shim that usually hotplugs its vCPUs. We either need to restrict this action for boot CPU only or allow secondary CPUs to modify forced CPU capabilities at runtime. Choose the former since modifying forced capabilities out of boot path leaves the system in potentially inconsistent state. Signed-off-by: Igor Druzhinin I've committed this to unstable, as per the outcome of the community call. What about this for 4.13? Iirc the breakage was introduced during this development cycle. In this case: Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 1/2] xenbus/backend: Add memory pressure handler callback
On Tue, Dec 10, 2019 at 11:21 AM Roger Pau Monné wrote:
>
> On Tue, Dec 10, 2019 at 11:16:35AM +0100, Roger Pau Monné wrote:
> > On Tue, Dec 10, 2019 at 08:06:27AM +, SeongJae Park wrote:
> > > diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> > > index 869c816d5f8c..cdb075e4182f 100644
> > > --- a/include/xen/xenbus.h
> > > +++ b/include/xen/xenbus.h
> > > @@ -104,6 +104,7 @@ struct xenbus_driver {
> > > struct device_driver driver;
> > > int (*read_otherend_details)(struct xenbus_device *dev);
> > > int (*is_ready)(struct xenbus_device *dev);
> > > + unsigned (*reclaim)(struct xenbus_device *dev);
> >
> > ... hence I wonder why it's returning an unsigned when it's just
> > ignored.
> >
> > IMO it should return an int to signal errors, and the return should be
> > ignored.
>
> Meant to write 'shouldn't be ignored' sorry.
Thanks for good opinions and comments! I will apply your comments in the next
version.
Thanks,
SeongJae Park
>
> Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 1/2] xenbus/backend: Add memory pressure handler callback
On Tue, Dec 10, 2019 at 11:16:35AM +0100, Roger Pau Monné wrote:
> On Tue, Dec 10, 2019 at 08:06:27AM +, SeongJae Park wrote:
> > diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> > index 869c816d5f8c..cdb075e4182f 100644
> > --- a/include/xen/xenbus.h
> > +++ b/include/xen/xenbus.h
> > @@ -104,6 +104,7 @@ struct xenbus_driver {
> > struct device_driver driver;
> > int (*read_otherend_details)(struct xenbus_device *dev);
> > int (*is_ready)(struct xenbus_device *dev);
> > + unsigned (*reclaim)(struct xenbus_device *dev);
>
> ... hence I wonder why it's returning an unsigned when it's just
> ignored.
>
> IMO it should return an int to signal errors, and the return should be
> ignored.
Meant to write 'shouldn't be ignored' sorry.
Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86 / iommu: set up a scratch page in the quarantine domain
> -Original Message- > From: Jan Beulich > Sent: 10 December 2019 09:45 > To: Durrant, Paul > Cc: Jürgen Groß ; Tian, Kevin ; > Andrew Cooper ; xen-devel@lists.xenproject.org; > Roger Pau Monné ; Wei Liu > Subject: Re: [PATCH v2] x86 / iommu: set up a scratch page in the > quarantine domain > > On 10.12.2019 10:16, Durrant, Paul wrote: > >> -Original Message- > >> From: Jürgen Groß > >> Sent: 10 December 2019 09:07 > >> To: Jan Beulich > >> Cc: Tian, Kevin ; Durrant, Paul > >> ; Andrew Cooper ; xen- > >> de...@lists.xenproject.org; Roger Pau Monné ; Wei > >> Liu > >> Subject: Re: [PATCH v2] x86 / iommu: set up a scratch page in the > >> quarantine domain > >> > >> On 10.12.19 09:57, Jan Beulich wrote: > >>> On 10.12.2019 09:12, Jürgen Groß wrote: > On 10.12.19 09:05, Jan Beulich wrote: > > On 10.12.2019 08:16, Tian, Kevin wrote: > >> While the quarantine idea sounds good overall, I'm still not > >> convinced > >> to have it the only way in place just for handling some known-buggy > >> device. It kills the possibility of identifying a new buggy device > >> and then > >> deciding not to use it in the first space... I thought about > whether > >> it > >> will get better when future IOMMU implements A/D bit - by checking > >> access bit being set then we'll know some buggy device exists, but, > >> the scratch page is shared by all devices then we cannot rely on > this > >> feature to find out the actual buggy one. > > > > Thinking about it - yes, I think I agree. This (as with so many > > workarounds) would better be an off-by-default one. The main issue > > I understand this would have is that buggy systems then might hang > > without even having managed to get a log message out - Paul? > > > > Jürgen - would you be amenable to an almost last minute refinement > > here (would then also need to still be backported to 4.12.2, or > > the original backport reverted, to avoid giving the impression of > > a regression)? > > So what is your suggestion here? To have a boot option (defaulting to > off) for enabling the scratch page? > >>> > >>> Yes (and despite having seen Paul's reply). > >> > >> I'd release ack such a patch in case you come to an agreement regarding > >> the default soon. > >> > > > > Ok. The default is not that crucial. Perhaps it's just me who thinks > > defaults should be chosen on the basis of being most likely to result > > in a working system. > > If it wasn't for quirky hardware (or firmware to cover the general case, > in particular to avoid getting quoted on this wrt my position on EFI > workarounds), I'd agree. But personally I think Kevin's point takes > priority here: Admins should at least be aware of running quirky > hardware, and hence I'd prefer the default to be logging of faults > rather than their silencing. Documentation of the new (sub-)option may > give suitable hints, and we may even go as far as providing a Kconfig > option for the default to be chosen at build time. > > Main question now is - who's going to make a patch? Will you? Should I? > I'm happy to do it, but it would probably be more expedient if you did. Paul > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] x86: do not enable global pages when virtualized on AMD hardware
On Tue, Dec 10, 2019 at 11:11:18AM +0100, Jan Beulich wrote:
> On 09.12.2019 18:37, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/pv/domain.c
> > +++ b/xen/arch/x86/pv/domain.c
> > @@ -118,6 +118,19 @@ unsigned long pv_fixup_guest_cr4(const struct vcpu *v,
> > unsigned long cr4)
> > (mmu_cr4_features & PV_CR4_GUEST_VISIBLE_MASK));
> > }
> >
> > +static int8_t __read_mostly opt_global_pages = -1;
> > +boolean_runtime_param("global-pages", opt_global_pages);
> > +
> > +static int __init pge_init(void)
> > +{
> > +if ( opt_global_pages == -1 )
> > +opt_global_pages = !cpu_has_hypervisor ||
> > + boot_cpu_data.x86_vendor != X86_VENDOR_AMD;
>
> I was about to commit this when I noticed - what about Hygon here?
Oh the vendor ID is different albeit it's just a clone. Please feel
free to add it at commit.
I also wonder: it might be good to have some kind of macro that
matches both AMD and Hygon (IS_AMD_COMPAT or some such) in order to
avoid this kind of mistakes in the future.
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 1/2] xenbus/backend: Add memory pressure handler callback
On Tue, Dec 10, 2019 at 08:06:27AM +, SeongJae Park wrote:
> Granting pages consumes backend system memory. In systems configured
> with insufficient spare memory for those pages, it can cause a memory
> pressure situation. However, finding the optimal amount of the spare
> memory is challenging for large systems having dynamic resource
> utilization patterns. Also, such a static configuration might lack a
s/lack a/lack/
> flexibility.
>
> To mitigate such problems, this commit adds a memory reclaim callback to
> 'xenbus_driver'. Using this facility, 'xenbus' would be able to monitor
> a memory pressure and request specific devices of specific backend
s/monitor a/monitor/
> drivers which causing the given pressure to voluntarily release its
...which are causing...
> memory.
>
> That said, this commit simply requests every callback registered driver
> to release its memory for every domain, rather than issueing the
s/issueing/issuing/
> requests to the drivers and the domain in charge. Such things will be
I'm afraid I don't understand the "domain in charge" part of this
sentence.
> done in a futur. Also, this commit focuses on memory only. However, it
... done in a future change. Also I think the period after only should
be removed in order to tie both sentences together.
> would be ablt to be extended for general resources.
s/ablt/able/
>
> Signed-off-by: SeongJae Park
> ---
> drivers/xen/xenbus/xenbus_probe_backend.c | 31 +++
> include/xen/xenbus.h | 1 +
> 2 files changed, 32 insertions(+)
>
> diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c
> b/drivers/xen/xenbus/xenbus_probe_backend.c
> index b0bed4faf44c..5a5ba29e39df 100644
> --- a/drivers/xen/xenbus/xenbus_probe_backend.c
> +++ b/drivers/xen/xenbus/xenbus_probe_backend.c
> @@ -248,6 +248,34 @@ static int backend_probe_and_watch(struct notifier_block
> *notifier,
> return NOTIFY_DONE;
> }
>
> +static int xenbus_backend_reclaim(struct device *dev, void *data)
> +{
> + struct xenbus_driver *drv;
Newline and const.
> + if (!dev->driver)
> + return -ENOENT;
> + drv = to_xenbus_driver(dev->driver);
> + if (drv && drv->reclaim)
> + drv->reclaim(to_xenbus_device(dev));
You seem to completely ignore the return of the reclaim hook...
> + return 0;
> +}
> +
> +/*
> + * Returns 0 always because we are using shrinker to only detect memory
> + * pressure.
> + */
> +static unsigned long xenbus_backend_shrink_count(struct shrinker *shrinker,
> + struct shrink_control *sc)
> +{
> + bus_for_each_dev(_backend.bus, NULL, NULL,
> + xenbus_backend_reclaim);
> + return 0;
> +}
> +
> +static struct shrinker xenbus_backend_shrinker = {
> + .count_objects = xenbus_backend_shrink_count,
> + .seeks = DEFAULT_SEEKS,
> +};
> +
> static int __init xenbus_probe_backend_init(void)
> {
> static struct notifier_block xenstore_notifier = {
> @@ -264,6 +292,9 @@ static int __init xenbus_probe_backend_init(void)
>
> register_xenstore_notifier(_notifier);
>
> + if (register_shrinker(_backend_shrinker))
> + pr_warn("shrinker registration failed\n");
> +
> return 0;
> }
> subsys_initcall(xenbus_probe_backend_init);
> diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> index 869c816d5f8c..cdb075e4182f 100644
> --- a/include/xen/xenbus.h
> +++ b/include/xen/xenbus.h
> @@ -104,6 +104,7 @@ struct xenbus_driver {
> struct device_driver driver;
> int (*read_otherend_details)(struct xenbus_device *dev);
> int (*is_ready)(struct xenbus_device *dev);
> + unsigned (*reclaim)(struct xenbus_device *dev);
... hence I wonder why it's returning an unsigned when it's just
ignored.
IMO it should return an int to signal errors, and the return should be
ignored.
Also, I think it would preferable for this function to take an extra
parameter to describe the resource the driver should attempt to free
(ie: memory or interrupts for example). I'm however not able to find
any existing Linux type to describe such resources.
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] Remove myself as vm_event maintainer
On 12/10/19 11:57 AM, Jan Beulich wrote: > On 08.12.2019 11:07, Razvan Cojocaru wrote: >> --- >> MAINTAINERS | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 9c827ad759..012c847ebd 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -428,7 +428,6 @@ L: xen-devel@lists.xenproject.org >> F: unmodified_drivers/linux-2.6/ >> >> VM EVENT, MEM ACCESS and MONITOR >> -M: Razvan Cojocaru >> M: Tamas K Lengyel >> R: Alexandru Isaila >> R: Petre Pircalabu > > No matter the contents, I guess this still needs an S-o-b of yours. Re-sent, sorry for the inconvenience. Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4] x86: do not enable global pages when virtualized on AMD hardware
On 09.12.2019 18:37, Roger Pau Monne wrote:
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -118,6 +118,19 @@ unsigned long pv_fixup_guest_cr4(const struct vcpu *v,
> unsigned long cr4)
> (mmu_cr4_features & PV_CR4_GUEST_VISIBLE_MASK));
> }
>
> +static int8_t __read_mostly opt_global_pages = -1;
> +boolean_runtime_param("global-pages", opt_global_pages);
> +
> +static int __init pge_init(void)
> +{
> +if ( opt_global_pages == -1 )
> +opt_global_pages = !cpu_has_hypervisor ||
> + boot_cpu_data.x86_vendor != X86_VENDOR_AMD;
I was about to commit this when I noticed - what about Hygon here?
I'm happy to make the adjustment while committing, but I don't
want to do so without your consent.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] x86/AMD: unbreak CPU hotplug on AMD systems without RstrFpErrPtrs
On 04.12.2019 00:56, Igor Druzhinin wrote: > If the feature is not present Xen will try to force X86_BUG_FPU_PTRS > feature at CPU identification time. This is especially noticeable in > PV-shim that usually hotplugs its vCPUs. We either need to restrict this > action for boot CPU only or allow secondary CPUs to modify > forced CPU capabilities at runtime. Choose the former since modifying > forced capabilities out of boot path leaves the system in potentially > inconsistent state. > > Signed-off-by: Igor Druzhinin I've committed this to unstable, as per the outcome of the community call. What about this for 4.13? Iirc the breakage was introduced during this development cycle. Jan > --- > Changes in v2: > - pick the former approach instead of the latter > --- > xen/arch/x86/cpu/amd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c > index fec2830..8b5f0f2 100644 > --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -583,7 +583,7 @@ static void init_amd(struct cpuinfo_x86 *c) >* Older AMD CPUs don't save/load FOP/FIP/FDP unless an FPU exception >* is pending. Xen works around this at (F)XRSTOR time. >*/ > - if (!cpu_has(c, X86_FEATURE_RSTR_FP_ERR_PTRS)) > + if (c == _cpu_data && !cpu_has(c, X86_FEATURE_RSTR_FP_ERR_PTRS)) > setup_force_cpu_cap(X86_BUG_FPU_PTRS); > > /* > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH V2] Remove myself as vm_event maintainer
Signed-off-by: Razvan Cojocaru --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 9c827ad759..012c847ebd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -428,7 +428,6 @@ L: xen-devel@lists.xenproject.org F: unmodified_drivers/linux-2.6/ VM EVENT, MEM ACCESS and MONITOR -M: Razvan Cojocaru M: Tamas K Lengyel R: Alexandru Isaila R: Petre Pircalabu -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: store cr4 during suspend/resume
On 10.12.2019 10:59, Andrew Cooper wrote: > On 09/12/2019 18:06, Roger Pau Monne wrote: >> Currently cr4 is not cached before suspension, and mmu_cr4_features is >> used in order to restore the expected cr4 value. This is correct so >> far because the tasklet that executes the suspend/resume code is >> running in the idle vCPU context. >> >> In order to make the code less fragile, explicitly save the current >> cr4 value before suspension, so that it can be restored afterwards. >> This ensures that the cr4 value cached in the cpu_info doesn't get out >> of sync after resume from suspension. >> >> Suggested-by: Jan Beulich >> Signed-off-by: Roger Pau Monné > > Why? There is nothing fragile here. Suspend/resume is always in idle > context and loads of other logic already depends on this. > > I've been slowly stripping out redundant saved state like this. Where it's clearly redundant, this is fine. But I don't think it's sufficiently clear here, and going back to what was there before is imo generally less error prone than going to some fixed state. Furthermore I was hoping we could eventually do away with mmu_cr4_features. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/mm: factor out the code for shattering an l3 PTE
On Mon, 2019-12-09 at 14:22 +0100, Jan Beulich wrote:
> On 07.12.2019 19:20, Xia, Hongyan wrote:
> > On Fri, 2019-12-06 at 17:50 +, Andrew Cooper wrote:
> > > On 06/12/2019 15:53, Hongyan Xia wrote:
> > > > +/* Shatter an l3 entry and populate l2. If virt is passed in,
> > > > also
> > > > do flush. */
> > > > +static void shatter_l3e(l3_pgentry_t *pl3e, l2_pgentry_t *l2t,
> > > > +unsigned long virt, bool locking)
> > > > +{
> > > > +unsigned int i;
> > > > +l3_pgentry_t ol3e = *pl3e;
> > > > +
> > > > +for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
> > > > +l2e_write(l2t + i,
> > > > + l2e_from_pfn(l3e_get_pfn(ol3e) +
> > > > + (i << PAGETABLE_ORDER),
> > > > + l3e_get_flags(ol3e)));
> > >
> > > The PTE macros are especially poor for generated asm, and in
> > > cases
> > > like
> > > this, I'd like to improve things.
> > >
> > > In particular, I believe the following code has identical
> > > behaviour:
> > >
> > > l2_pgentry_t nl2e = l2e_from_intpte(l3e_get_intpte(ol3e));
> > >
> > > for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++, nl2e.l2 +=
> > > PAGETABLE_ORDER )
> > > l2e_write(l2t + i, nl2e);
> > >
> > > (although someone please double check my logic) and rather better
> > > asm
> > > generation. (I also expect there to be some discussion on
> > > whether
> > > using
> > > n2le.l2 directly is something we'd want to start doing.)
> > >
> >
> > I believe it should be nl2e.l2 += 1<<(PAGETABLE_ORDER+PAGE_SHIFT) ?
>
> Indeed.
>
> > Although the code rarely touches the field (.l2) directly, so maybe
> > use
> > the macros (get_intpte -> add -> from_intpte) for consistency? They
> > should produce the same code if the compiler is not too stupid.
>
> I think the to/from intpte transformations should be used sparingly
> too (as being dangerous). How about we make PTEs proper unions, with
> a full-field intpte_t as we have it now combined with a set of bit
> fields? This would at least eliminate the need for using PAGE_SHIFT
> in constructs like the above.
I can see this makes the code look much nicer. One concern I have is
that Andrew's suggestion was to improve the generated assembly code,
and using packed bit fields may generate even more masking and bit
shifting, which in the end might give us more assembly code than before
the refactoring.
Hongyan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] docs/sphinx: How Xen Boots on x86
On 10.12.2019 10:55, Andrew Cooper wrote: > On 10/12/2019 07:52, Jan Beulich wrote: >> On 09.12.2019 17:42, Andrew Cooper wrote: >>> On 09/12/2019 15:20, Jan Beulich wrote: On 06.12.2019 20:34, Andrew Cooper wrote: > +Objects > +~~~ > + > +To begin with, most object files are compiled and linked. This includes > the > +Multiboot 1 and 2 headers and entrypoints, including the Multiboot 2 > tags for > +EFI extensions. When ``CONFIG_PVH_GUEST`` is selected at build time, > this > +includes the PVH entrypoint and associated ELF notes. > + > +Depending on whether the compiler supports > ``__attribute__((__ms_abi__))`` or > +not, either an EFI stub is included which nops/fails applicable setup > calls, > +or full EFI support is included. Perhaps also mention that the linker needs to support the necessary binary output format? And perhaps "setup and runtime calls"? >>> Link time behaviour is (deliberately) in a later section. >> I realize(d) this, but the statement above is simply not true without >> also mentioning required linker capabilities: The object files won't >> have "full EFI support included" in this case. So I'd expect a "see >> also" here at the very least. > > Note how XEN_BUILD_EFI and XEN_BUILD_PE are different, one by compiler > support for ms_abi, and one by linker support for i386pep. > > Linker support for i386pep is not required at all to get EFI support in > Xen. This is how the MB2+EFI path is constructed. Hmm, indeed. Meaning the build reporting "EFI support disabled" has been wrong since the splitting of the two. Should now be something like "Not generating xen.efi", I guess. With the minor re-stating of 32-bit mode and the PE32+ naming adjustment then Reviewed-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: store cr4 during suspend/resume
On 09/12/2019 18:06, Roger Pau Monne wrote: > Currently cr4 is not cached before suspension, and mmu_cr4_features is > used in order to restore the expected cr4 value. This is correct so > far because the tasklet that executes the suspend/resume code is > running in the idle vCPU context. > > In order to make the code less fragile, explicitly save the current > cr4 value before suspension, so that it can be restored afterwards. > This ensures that the cr4 value cached in the cpu_info doesn't get out > of sync after resume from suspension. > > Suggested-by: Jan Beulich > Signed-off-by: Roger Pau Monné Why? There is nothing fragile here. Suspend/resume is always in idle context and loads of other logic already depends on this. I've been slowly stripping out redundant saved state like this. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
