Re: [Xen-devel] [PATCH v3 11/15] xsm, argo: XSM control for argo register

[Christopher Clark]

On Mon, Jan 7, 2019 at 3:07 PM DeGraaf, Daniel G  wrote:
>
> > From: Christopher Clark 
> > Subject: [PATCH v3 11/15] xsm, argo: XSM control for argo register
> >
> > XSM controls for argo ring registration with two distinct cases, where
> > the ring being registered is:
> >
> > 1) Single source:  registering a ring for communication to receive messages
> >from a specified single other domain.
> >Default policy: allow.
> >
> > 2) Any source: registering a ring for communication to receive messages
> >from any, or all, other domains (ie. wildcard).
> >Default policy: deny, with runtime policy configuration via bootparam.
> >
> > The existing argo-mac boot parameter indicates administrator preference for
> > either permissive or strict access control, which will allow or deny
> > registration of any-sender rings.
> >
> > This commit modifies the signature of core XSM hook functions in order to
> > apply 'const' to arguments, needed in order for 'const' to be accepted in
> > signature of functions that invoke them.
> >
> > Signed-off-by: Christopher Clark 
>
> Acked-by: Daniel De Graaf 
>
> While it does not need to be a part of this patch, somewhere in the series 
> you should add a rule allowing these features to be used by guests in the 
> default XSM policy; tools/flask/policy/modules/guest_features.te is where 
> features like this have previously been handled.  Since you're adding 
> permissions one at a time, you could add the rules all at once or as a part 
> of the patch adding the vector.

Thanks for the reviews, acks and pointer to the policy file. I will
add to the default XSM policy in the next revision that I post.

thanks,

Christopher

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 09/15] argo: implement the sendv op; evtchn: expose send_guest_global_virq

[Christopher Clark]

Thanks for the review, Roger. Replies inline below.

On Wed, Jan 9, 2019 at 10:57 AM Roger Pau Monné  wrote:
>
>  to.On Mon, Jan 7, 2019 at 8:44 AM Christopher Clark
>  wrote:
> >
> > sendv operation is invoked to perform a synchronous send of buffers
> > contained in iovs to a remote domain's registered ring.
> >
> > diff --git a/xen/common/argo.c b/xen/common/argo.c
> > index 59ce8c4..4548435 100644
> > --- a/xen/common/argo.c
> > +++ b/xen/common/argo.c

> >
> > +static int
> > +memcpy_to_guest_ring(struct argo_ring_info *ring_info, uint32_t offset,
> > + const void *src, XEN_GUEST_HANDLE(uint8_t) src_hnd,
> > + uint32_t len)
> > +{
> > +unsigned int mfns_index = offset >> PAGE_SHIFT;
> > +void *dst;
> > +int ret;
> > +unsigned int src_offset = 0;
> > +
> > +ASSERT(spin_is_locked(_info->lock));
> > +
> > +offset &= ~PAGE_MASK;
> > +
> > +if ( (len > XEN_ARGO_MAX_RING_SIZE) || (offset > 
> > XEN_ARGO_MAX_RING_SIZE) )
> > +return -EFAULT;
> > +
> > +while ( (offset + len) > PAGE_SIZE )
>
> I think you could map the whole ring in contiguous virtual address
> space and then writing to it would be much more easy, you wouldn't
> need to iterate with memcpy or copy_from_guest, take a look at __vmap.
> You could likely map this when the ring gets setup and keep it mapped
> for the lifetime of the ring.

You're right about that, because map_domain_page_global, which the
current code uses, uses vmap itself. I think there's a couple of
reasons why the code has been implemented the iterative way though.

The first is that I think ring resize has been a consideration: it's
useful to be able to increase the size of a live and active ring that
is under load without having to tear down the mappings, find a new
virtual address region of the right size and then remap it: you can
just supply some more memory and map those pages onto the end of the
ring, and ensure both sides know about the new ring size. Similarly,
shrinking a quiet ring can be useful.
However, the "gfn race" that you (correctly) pointed out in an earlier
review, and Jan's related request to drop the "revalidate an existing
mapping on ring reregister" motivated removal of a section of the code
involved, and then in v3 of the series, I've actually just blocked
ring resize because it's missing a walk through the pending
notifications to find any that have become untriggerable with the new
ring size when a ring is shrunk and I'd like to defer implementing
that for now. So the ring resize reason is more of a consideration for
a possible later version of Argo than the current one.

The second reason is about avoiding exposing the Xen virtual memory
allocator directly to frequent guest-supplied size requests for
contiguous regions (of up to 16GB). With single-page allocations to
build a ring, fragmentation is not a problem, and mischief by a guest
seems difficult. Changing it to issue requests for contiguous regions,
with variable ring sizes up to the maximum of 16GB, it seems like
significant fragmentation may be achievable. I don't know the
practical impact of that but it seems worth avoiding. Are the other
users of __vmap (or vmap) for multi-gigabyte regions only either
boot-time, infrequent operations (livepatch), or for actions by
privileged (ie. somewhat trusted) domains (ioremap), or is it already
a frequent operation somewhere else?

Given the context above, and Jason's simplification to the
memcpy_to_guest_ring function, plus the imminent merge freeze
deadline, and the understanding that this loop and the related data
structures supporting it have been tested and are working, would it be
acceptable to omit making this contiguous mapping change from this
current series?

>
> > +{
> > +unsigned int head_len = PAGE_SIZE - offset;
> > +
> > +ret = ring_map_page(ring_info, mfns_index, );
> > +if ( ret )
> > +return ret;
> > +
> > +if ( src )
> > +{
> > +memcpy(dst + offset, src + src_offset, head_len);
> > +src_offset += head_len;
> > +}
> > +else
> > +{
> > +ret = copy_from_guest(dst + offset, src_hnd, head_len) ?
> > +-EFAULT : 0;
> > +if ( ret )
> > +return ret;
>
> You can simplify this to:
>
> if ( copy_from_guest(...) )
> return -EFAULT;

yes! ack - thanks


> > +/*
> > + * get_sanitized_ring creates a modified copy of the ring pointers where
> > + * the rx_ptr is rounded up to ensure it is aligned, and then ring
> > + * wrap is handled. Simplifies safe use of the rx_ptr for available
> > + * space calculation.
> > + */
> > +static int
> > +get_sanitized_ring(xen_argo_ring_t *ring, struct argo_ring_info *ring_info)
> > +{
> > +uint32_t rx_ptr;
> > +int ret;
> > +
> > +ret = get_rx_ptr(ring_info, _ptr);
> > +if ( ret )
> > +return ret;
> > +
> > +ring->tx_ptr = ring_info->tx_ptr;

Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL

[Julien Grall]

Hi,

Sorry for the formatting.

On Wed, 9 Jan 2019, 18:43 Stefano Stabellini, 
wrote:

> Introduce a macro, SYMBOL, which is similar to RELOC_HIDE, but it is
> meant to be used everywhere symbols such as _stext and _etext are used
> in the code. It can take an array type as a parameter, and it returns
> the same type.
>
> SYMBOL is needed when accessing symbols such as _stext and _etext
> because the C standard forbids for both comparisons and substraction
> (see C Standard, 6.5.6 [ISO/IEC 9899:2011] and [1]) between pointers
> pointing to different objects. _stext, _etext, etc. are all pointers to
> different objects from ANCI C point of view.
>

This does not make sense because you still return a pointer and therefore
the undefined behavior is still present.

I really don't believe this patch is going to make the MISRA tool happy.
Furthermore, IIRC, Linux to returns unsigned long. So I would like to
understand why the trick is no needed for us...

At that stage, we should probably involve MlSRA folks (PRQA) to have a
better understanding on what is expected.

Cheers,


> To work around potential C compiler issues (which have actually
> been found, see the comment on top of RELOC_HIDE in Linux), and to help
> with certifications, let's introduce some syntactic sugar to be used in
> following patches.


> [1]
> https://wiki.sei.cmu.edu/confluence/display/c/ARR36-C.+Do+not+subtract+or+compare+two+pointers+that+do+not+refer+to+the+same+array
>
> Signed-off-by: Stefano Stabellini 
> CC: jbeul...@suse.com
> CC: andrew.coop...@citrix.com
> CC: wei.l...@citrix.com
> ---
> Changes in v6:
> - drop acks
> - don't use RELOC_HIDE for the implementation
> - return native type from SYMBOL
>
> Changes in v4:
> - add acked-bys
> - remove unneeded parenthesis
>
> Changes in v3:
> - improve commit message
> - rename __symbol to SYMBOL to avoid name space violations
>
> Changes in v2:
> - do not cast return to char*
> - move to common header
> ---
>  xen/include/xen/compiler.h | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index ff6c0f5..d4c856c 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -99,6 +99,16 @@
>  __asm__ ("" : "=r"(__ptr) : "0"(ptr));  \
>  (typeof(ptr)) (__ptr + (off)); })
>
> +/*
> + * Similar to RELOC_HIDE, but written to be used with symbols such as
> + * _stext and _etext to avoid undefined behavior comparing pointers to
> + * different objects. It can handle array types.
> + */
> +#define SYMBOL(ptr)   \
> +  ({ unsigned long __ptr;   \
> +__asm__ ("" : "=r"(__ptr) : "0"(ptr));  \
> +(typeof(*(ptr)) *) (__ptr); })
> +
>  #ifdef __GCC_ASM_FLAG_OUTPUTS__
>  # define ASM_FLAG_OUT(yes, no) yes
>  #else
> --
> 1.9.1
>
>
> ___
> 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

[Xen-devel] [linux-4.19 test] 131859: regressions - FAIL

[osstest service owner]

flight 131859 linux-4.19 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131859/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. 
vs. 129313
 test-amd64-amd64-amd64-pvgrub  7 xen-bootfail REGR. vs. 129313
 test-amd64-amd64-xl-shadow7 xen-boot fail REGR. vs. 129313
 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host   fail REGR. vs. 129313
 test-amd64-amd64-i386-pvgrub  7 xen-boot fail REGR. vs. 129313
 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host   fail REGR. vs. 129313
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 129313
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. 
vs. 129313
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 129313
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 129313
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 129313
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 129313
 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 129313
 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 129313
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 129313
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 129313
 test-amd64-amd64-examine  8 reboot   fail REGR. vs. 129313
 test-amd64-i386-qemut-rhel6hvm-amd 13 guest-start.2 fail in 131739 REGR. vs. 
129313

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-rumprun-amd64 17 rumprun-demo-xenstorels/xenstorels.repeat 
fail in 131642 pass in 131859
 test-amd64-amd64-xl-qcow2 19 guest-start/debian.repeat fail in 131739 pass in 
131859
 test-armhf-armhf-libvirt  6 xen-install  fail in 131791 pass in 131859
 test-amd64-i386-xl7 xen-boot   fail pass in 131642
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail pass in 
131739
 test-amd64-i386-freebsd10-i386  7 xen-boot fail pass in 131791
 test-amd64-i386-rumprun-i386 17 rumprun-demo-xenstorels/xenstorels.repeat fail 
pass in 131791
 test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail pass in 
131791

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  7 xen-boot   fail never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   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 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-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-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 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-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 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-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 

Re: [Xen-devel] [PATCH v3 09/15] argo: implement the sendv op; evtchn: expose send_guest_global_virq

[Christopher Clark]

On Wed, Jan 9, 2019 at 10:05 AM Jason Andryuk  wrote:
>
> On Mon, Jan 7, 2019 at 2:43 AM Christopher Clark
>  wrote:
>
> 
>
> > @@ -342,6 +357,413 @@ update_tx_ptr(struct argo_ring_info *ring_info, 
> > uint32_t tx_ptr)
> >  smp_wmb();
> >  }
> >
> > +static int
> > +memcpy_to_guest_ring(struct argo_ring_info *ring_info, uint32_t offset,
> > + const void *src, XEN_GUEST_HANDLE(uint8_t) src_hnd,
> > + uint32_t len)
> > +{
> > +unsigned int mfns_index = offset >> PAGE_SHIFT;
> > +void *dst;
> > +int ret;
> > +unsigned int src_offset = 0;
> > +
> > +ASSERT(spin_is_locked(_info->lock));
> > +
> > +offset &= ~PAGE_MASK;
> > +
> > +if ( (len > XEN_ARGO_MAX_RING_SIZE) || (offset > 
> > XEN_ARGO_MAX_RING_SIZE) )
> > +return -EFAULT;
> > +
> > +while ( (offset + len) > PAGE_SIZE )
> > +{
> > +unsigned int head_len = PAGE_SIZE - offset;
>
> I think this while loop could be re-written as
> while (len) {
> head_len = len > PAGE_SIZE ? PAGE_SIZE - offset: len;
>
> and then the extra copying below outside the loop could be dropped.
>
> The first loop does a partial copy at offset and then sets offset=0.
> The next N loops copy exactly PAGE_SIZE.
> The Final copy does the remaining len bytes.

That looks right to me and makes a nice simplification to that
function -- thanks.

> 
>
> > +
> > +/*
> > + * iov_count returns its count on success via an out variable to avoid
> > + * potential for a negative return value to be used incorrectly
> > + * (eg. coerced into an unsigned variable resulting in a large incorrect 
> > value)
> > + */
> > +static int
> > +iov_count(const xen_argo_iov_t *piov, unsigned long niov, uint32_t *count)
> > +{
> > +uint32_t sum_iov_lens = 0;
> > +
> > +if ( niov > XEN_ARGO_MAXIOV )
> > +return -EINVAL;
> > +
> > +while ( niov-- )
> > +{
> > +/* valid iovs must have the padding field set to zero */
> > +if ( piov->pad )
> > +{
> > +argo_dprintk("invalid iov: padding is not zero\n");
> > +return -EINVAL;
> > +}
> > +
> > +/* check each to protect sum against integer overflow */
> > +if ( piov->iov_len > XEN_ARGO_MAX_RING_SIZE )
>
> Should this be MAX_ARGO_MESSAGE_SIZE?  MAX_ARGO_MESSAGE_SIZE is less
> than XEN_ARGO_MAX_RING_SIZE, so we can pass this check and then just
> fail the one below.

ack - I'll switch it to MAX_ARGO_MESSAGE_SIZE.

> 
>
> > @@ -1073,6 +1683,49 @@ do_argo_op(unsigned int cmd, 
> > XEN_GUEST_HANDLE_PARAM(void) arg1,
> >  break;
> >  }
> >
> > +case XEN_ARGO_OP_sendv:
> > +{
> > +xen_argo_send_addr_t send_addr;
> > +
> > +XEN_GUEST_HANDLE_PARAM(xen_argo_send_addr_t) send_addr_hnd =
> > +guest_handle_cast(arg1, xen_argo_send_addr_t);
> > +XEN_GUEST_HANDLE_PARAM(xen_argo_iov_t) iovs_hnd =
> > +guest_handle_cast(arg2, xen_argo_iov_t);
> > +/* arg3 is niov */
> > +/* arg4 is message_type. Must be a 32-bit value. */
> > +
> > +rc = copy_from_guest(_addr, send_addr_hnd, 1) ? -EFAULT : 0;
> > +if ( rc )
> > +break;
> > +
> > +if ( send_addr.src.domain_id == XEN_ARGO_DOMID_ANY )
> > +send_addr.src.domain_id = currd->domain_id;
> > +
> > +/* No domain is currently authorized to send on behalf of another 
> > */
> > +if ( unlikely(send_addr.src.domain_id != currd->domain_id) )
> > +{
> > +rc = -EPERM;
> > +break;
> > +}
> > +
> > +/* Reject niov or message_type values that are outside 32 bit 
> > range. */
> > +if ( unlikely((arg3 > XEN_ARGO_MAXIOV) || (arg4 & ~0xUL)) )
> > +{
> > +rc = -EINVAL;
> > +break;
> > +}
>
> This needs to check send_addr.src.pad and send_addr.dst.pad == 0.
> sendv() does not check the padding either.

ack - will fix.

thanks

Christopher

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [Qemu-devel] [PATCH 3/3] machine: Use shorter format for GlobalProperty arrays

[Eduardo Habkost]

On Tue, Jan 08, 2019 at 11:20:12AM +0100, Cornelia Huck wrote:
> On Tue, 8 Jan 2019 07:45:43 +0100
> Gerd Hoffmann  wrote:
> 
> >   Hi,
> > 
> > > +{ "migration", "decompress-error-check", "off" },
> > > +{ "hda-audio", "use-timer", "false" },
> > > +{ "cirrus-vga", "global-vmstate", "true" },
> > > +{ "VGA", "global-vmstate", "true" },
> > > +{ "vmware-svga", "global-vmstate", "true" },
> > > +{ "qxl-vga", "global-vmstate", "true" },  
> > 
> > I'd like to have the fields aligned.  Especially in cases like this one
> > where multiple devices get the same value assigned it makes things more
> > readable:
> > 
> > { "migration",   "decompress-error-check", "off"   },
> > { "hda-audio",   "use-timer",  "false" },
> > { "cirrus-vga",  "global-vmstate", "true"  },
> > { "VGA", "global-vmstate", "true"  },
> > { "vmware-svga", "global-vmstate", "true"  },
> > { "qxl-vga", "global-vmstate", "true"  },
> > 
> > thanks,
> >   Gerd
> > 
> 
> I'm a bit on the fence here. It does make things more readable (at
> least in your example), but I find editing aligned tables a bit
> annoying. OTOH, that won't happen often, anyway.

I'm unsure, too.  Also, not merging this series is increasing the
likelihood of conflicts with other patches.  I'm queueing this
version, and we can discuss alignment alternatives later.

(I'm less worried about conflicts caused by future alignment
patches because alignment conflicts are easier to sort out than
redoing the .driver/.property/.value conversion).

-- 
Eduardo

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 2/2] xen: use SYMBOL when required

[Stefano Stabellini]

On Wed, 9 Jan 2019, Jan Beulich wrote:
> >>> On 08.01.19 at 19:08,  wrote:
> > On Tue, 8 Jan 2019, Stefano Stabellini wrote:
> >> So, this is what I am going to do: I'll send a series update according
> >> to your suggestion, with SYMBOL returning the native pointer type. As I
> >> wrote earlier, although weaker, it is still an improvement from my point
> >> of view.
> > 
> > There is a problem with this though I didn't foresee :-(
> > 
> > The native type of _start is not char* -- it is char[]. So I cannot
> > actually return the native type from SYMBOL because I cannot cast to
> > (char[]). I didn't notice it until I actually tried it.
> > 
> > See the implementation of RELOC_HIDE:
> > 
> >   #define RELOC_HIDE(ptr, off)\
> > ({ unsigned long __ptr;   \
> >   __asm__ ("" : "=r"(__ptr) : "0"(ptr));  \
> >   (typeof(ptr)) (__ptr + (off)); })
> > 
> > It casts to the type at the end, the error is:
> > 
> >   error: cast specifies array type
> >(typeof(ptr)) (__ptr + (off)); })
> > 
> > We have a few options:
> > 
> > 1) use unsigned long as in this version of the series (the Linux kernel
> > also uses this technique)
> > Sorry if I insist, it is still the best I think :-)
> > 
> > 2) casts the parameters of SYMBOL to the corresponding pointer type
> > For instance:
> >   SYMBOL((char *)_start)
> >   SYMBOL((struct alt_instr *)__alt_instructions_end)
> > This works, but it is ugly, I would say it makes the code worse than
> > option 1)
> > 
> > 2) always return void* from SYMBOL
> > I don't think it is a good idea to use void* as a workaround here
> > 
> > 3) pass the desired return type to SYMBOL
> > For instance:
> >   SYMBOL(_start, char *)
> >   SYMBOL(__alt_instructions_end, struct alt_instr *)
> > Then SYMBOL would automatically cast the return type to char * and
> > struct alt_instr * according to the second parameter.
> > 
> > Do you have any other suggestions?
> 
> 4) 
> 
> #define RELOC_HIDE(ptr, off)\
> ({ unsigned long ptr_;   \
>   __asm__ ("" : "=r"(ptr_) : "0" (ptr));  \
>   (typeof(*(ptr)) *) (ptr_ + (off)); })
> 
> or, if not suitable for RELOC_HIDE() itself, clone the macro into one
> that fits SYMBOL()'s needs.

OK. I still don't think that this is a good idea. Nonetheless, I have
just sent a patch series that uses this trick in the implementation of
SYMBOL to return the native type.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v6 3/4] xen/x86: use SYMBOL when required

[Stefano Stabellini]

Use SYMBOL in cases of comparisons and subtractions of:

_start, _end, __2M_rwdata_start, __2M_rwdata_end, _stext, _etext,
__end_vpci_array, __start_vpci_array, _stextentry, _etextentry,
__trampoline_rel_start, __trampoline_rel_stop, __trampoline_seg_start,
__trampoline_seg_stop __per_cpu_start, __per_cpu_data_end

as by the C standard [1].

M3CM: Rule-18.2: Subtraction between pointers shall only be applied to
pointers that address elements of the same array

[1] 
https://wiki.sei.cmu.edu/confluence/display/c/ARR36-C.+Do+not+subtract+or+compare+two+pointers+that+do+not+refer+to+the+same+array

QAVerify: 2761
Signed-off-by: Stefano Stabellini 
CC: jbeul...@suse.com
CC: andrew.coop...@citrix.com
---
Changes in v6:
- more accurate commit message
- remove uneeded extra newline
- only use SYMBOL on problematic symbols in alternatives.c
- use new SYMBOL macro that returns the native type

Changes in v5:
- remove two spurious changes
- split into three patches
- remove SYMBOL() from derived variables
---
 xen/arch/x86/alternative.c  | 3 ++-
 xen/arch/x86/efi/efi-boot.h | 8 
 xen/arch/x86/percpu.c   | 8 
 xen/arch/x86/setup.c| 8 +---
 xen/arch/x86/smpboot.c  | 5 +++--
 xen/drivers/vpci/vpci.c | 2 +-
 6 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index b8c819a..92c54eb 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -273,7 +273,8 @@ static int __init nmi_apply_alternatives(const struct 
cpu_user_regs *regs,
 /* Disable WP to allow patching read-only pages. */
 write_cr0(cr0 & ~X86_CR0_WP);
 
-apply_alternatives(__alt_instructions, __alt_instructions_end);
+apply_alternatives(SYMBOL(__alt_instructions),
+   SYMBOL(__alt_instructions_end));
 
 write_cr0(cr0);
 
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 5789d2c..8dcd981 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -111,12 +111,12 @@ static void __init relocate_trampoline(unsigned long phys)
 return;
 
 /* Apply relocations to trampoline. */
-for ( trampoline_ptr = __trampoline_rel_start;
-  trampoline_ptr < __trampoline_rel_stop;
+for ( trampoline_ptr = SYMBOL(__trampoline_rel_start);
+  trampoline_ptr < SYMBOL(__trampoline_rel_stop);
   ++trampoline_ptr )
 *(u32 *)(*trampoline_ptr + (long)trampoline_ptr) += phys;
-for ( trampoline_ptr = __trampoline_seg_start;
-  trampoline_ptr < __trampoline_seg_stop;
+for ( trampoline_ptr = SYMBOL(__trampoline_seg_start);
+  trampoline_ptr < SYMBOL(__trampoline_seg_stop);
   ++trampoline_ptr )
 *(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys >> 4;
 }
diff --git a/xen/arch/x86/percpu.c b/xen/arch/x86/percpu.c
index 8be4ebd..920bb78 100644
--- a/xen/arch/x86/percpu.c
+++ b/xen/arch/x86/percpu.c
@@ -13,7 +13,7 @@ unsigned long __per_cpu_offset[NR_CPUS];
  * context of PV guests.
  */
 #define INVALID_PERCPU_AREA (0x8000L - (long)__per_cpu_start)
-#define PERCPU_ORDER get_order_from_bytes(__per_cpu_data_end - __per_cpu_start)
+#define PERCPU_ORDER get_order_from_bytes(SYMBOL(__per_cpu_data_end) - 
SYMBOL(__per_cpu_start))
 
 void __init percpu_init_areas(void)
 {
@@ -33,8 +33,8 @@ static int init_percpu_area(unsigned int cpu)
 if ( (p = alloc_xenheap_pages(PERCPU_ORDER, 0)) == NULL )
 return -ENOMEM;
 
-memset(p, 0, __per_cpu_data_end - __per_cpu_start);
-__per_cpu_offset[cpu] = p - __per_cpu_start;
+memset(p, 0, SYMBOL(__per_cpu_data_end) - SYMBOL(__per_cpu_start));
+__per_cpu_offset[cpu] = p - SYMBOL(__per_cpu_start);
 
 return 0;
 }
@@ -49,7 +49,7 @@ static void _free_percpu_area(struct rcu_head *head)
 {
 struct free_info *info = container_of(head, struct free_info, rcu);
 unsigned int cpu = info->cpu;
-char *p = __per_cpu_start + __per_cpu_offset[cpu];
+char *p = SYMBOL(__per_cpu_start) + __per_cpu_offset[cpu];
 
 free_xenheap_pages(p, PERCPU_ORDER);
 __per_cpu_offset[cpu] = INVALID_PERCPU_AREA;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 06eb483..5c35826 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -972,7 +972,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  * respective reserve_e820_ram() invocation below.
  */
 mod[mbi->mods_count].mod_start = virt_to_mfn(_stext);
-mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext;
+mod[mbi->mods_count].mod_end = SYMBOL(__2M_rwdata_end) - 
SYMBOL(_stext);
 }
 
 modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
@@ -1067,7 +1067,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  * data until after we have switched to the relocated pagetables!
  */
 barrier();
-move_memory(e + 

[Xen-devel] [PATCH v6 0/4] misc safety certification fixes

[Stefano Stabellini]

Hi all,

This version of the series addresses all the latest comments by Jan. The
principal change is to SYMBOL(), that now returns the native type,
instead of unsigned long.

I would like to note that I believe this not a good change. It would be
better, more safety compliant, to have SYMBOL() return unsigned long as
it was done in v5 of the series.

Cheers,

Stefano


The following changes since commit 808cff4c2af66afd61973451aeb7e708732abf90:

  sched/credit2: remove stale comment (2019-01-09 15:46:05 +0100)

are available in the git repository at:

  http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git 
certifications-6

for you to fetch changes up to 40ca1684137114698b13c18f2f1468e9bc75574f:

  xen/common: use SYMBOL when required (2019-01-09 15:36:21 -0800)


Stefano Stabellini (4):
  xen: introduce SYMBOL
  xen/arm: use SYMBOL when required
  xen/x86: use SYMBOL when required
  xen/common: use SYMBOL when required

 xen/arch/arm/alternative.c|  4 ++--
 xen/arch/arm/arm32/livepatch.c|  2 +-
 xen/arch/arm/arm64/livepatch.c|  2 +-
 xen/arch/arm/device.c |  6 +++---
 xen/arch/arm/livepatch.c  |  4 ++--
 xen/arch/arm/mm.c | 13 +++--
 xen/arch/arm/percpu.c |  8 
 xen/arch/arm/platform.c   |  6 --
 xen/arch/arm/setup.c  |  6 --
 xen/arch/x86/alternative.c|  3 ++-
 xen/arch/x86/efi/efi-boot.h   |  8 
 xen/arch/x86/percpu.c |  8 
 xen/arch/x86/setup.c  |  8 +---
 xen/arch/x86/smpboot.c|  5 +++--
 xen/common/kernel.c   |  8 ++--
 xen/common/lib.c  |  3 ++-
 xen/common/schedule.c |  6 --
 xen/common/spinlock.c |  4 +++-
 xen/common/version.c  |  6 +++---
 xen/common/virtual_region.c   | 12 ++--
 xen/drivers/vpci/vpci.c   |  2 +-
 xen/include/asm-arm/grant_table.h |  3 ++-
 xen/include/xen/compiler.h| 10 ++
 xen/include/xen/kernel.h  | 24 
 24 files changed, 95 insertions(+), 66 deletions(-)

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v6 4/4] xen/common: use SYMBOL when required

[Stefano Stabellini]

Use SYMBOL in cases of comparisons and subtractions of:

_start, _end, _stext, _etext, _srodata, _erodata, _sinittext,
_einittext, __note_gnu_build_id_start, __note_gnu_build_id_end,
__lock_profile_start, __lock_profile_end, __initcall_start,
__initcall_end, __presmp_initcall_end, __ctors_start, __ctors_end,
__end_schedulers_array, __start_schedulers_array, __start_bug_frames,
__stop_bug_frames_0, __stop_bug_frames_1, __stop_bug_frames_2,
__stop_bug_frames_3,

as by the C standard [1].

M3CM: Rule-18.2: Subtraction between pointers shall only be applied to
pointers that address elements of the same array

Since we are changing the body of is_kernel_text and friends, take the
opportunity to remove the leading underscores in the local variables
names, which are violationg namespace rules.

[1] 
https://wiki.sei.cmu.edu/confluence/display/c/ARR36-C.+Do+not+subtract+or+compare+two+pointers+that+do+not+refer+to+the+same+array

QAVerify: 2761
Signed-off-by: Stefano Stabellini 
CC: jbeul...@suse.com
CC: andrew.coop...@citrix.com
---
Changes in v6:
- more accurate commit message
- remove hard tabs
- remove leading underscores
- code style
- use SYMBOL only on the problematic bug_frames symbols
- use new SYMBOL macro that returns the native type

Changes in v5:
- remove two spurious changes
- split into three patches
- remove SYMBOL() from derived variables
---
 xen/common/kernel.c |  8 ++--
 xen/common/lib.c|  3 ++-
 xen/common/schedule.c   |  6 --
 xen/common/spinlock.c   |  4 +++-
 xen/common/version.c|  6 +++---
 xen/common/virtual_region.c | 12 ++--
 xen/include/xen/kernel.h| 24 
 7 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 5766a0f..ed913be 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -312,14 +312,18 @@ extern const initcall_t __initcall_start[], 
__presmp_initcall_end[],
 void __init do_presmp_initcalls(void)
 {
 const initcall_t *call;
-for ( call = __initcall_start; call < __presmp_initcall_end; call++ )
+for ( call = SYMBOL(__initcall_start);
+  call < SYMBOL(__presmp_initcall_end);
+  call++ )
 (*call)();
 }
 
 void __init do_initcalls(void)
 {
 const initcall_t *call;
-for ( call = __presmp_initcall_end; call < __initcall_end; call++ )
+for ( call = SYMBOL(__presmp_initcall_end);
+  call < SYMBOL(__initcall_end);
+  call++ )
 (*call)();
 }
 
diff --git a/xen/common/lib.c b/xen/common/lib.c
index 8ebec81..4e43ee5 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -497,7 +497,8 @@ extern const ctor_func_t __ctors_start[], __ctors_end[];
 void __init init_constructors(void)
 {
 const ctor_func_t *f;
-for ( f = __ctors_start; f < __ctors_end; ++f )
+
+for ( f = SYMBOL(__ctors_start); f < SYMBOL(__ctors_end); ++f )
 (*f)();
 
 /* Putting this here seems as good (or bad) as any other place. */
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index a957c5e..a81de40 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -67,8 +67,10 @@ DEFINE_PER_CPU(struct scheduler *, scheduler);
 /* Scratch space for cpumasks. */
 DEFINE_PER_CPU(cpumask_t, cpumask_scratch);
 
-extern const struct scheduler *__start_schedulers_array[], 
*__end_schedulers_array[];
-#define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
+extern const struct scheduler *__start_schedulers_array[],
+  *__end_schedulers_array[];
+#define NUM_SCHEDULERS (SYMBOL(__end_schedulers_array) - \
+SYMBOL(__start_schedulers_array))
 #define schedulers __start_schedulers_array
 
 static struct scheduler __read_mostly ops;
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 6bc52d7..ed1b2b0 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -474,7 +474,9 @@ static int __init lock_prof_init(void)
 {
 struct lock_profile **q;
 
-for ( q = &__lock_profile_start; q < &__lock_profile_end; q++ )
+for ( q = SYMBOL(&__lock_profile_start);
+  q < SYMBOL(&__lock_profile_end);
+  q++ )
 {
 (*q)->next = lock_profile_glb_q.elem_q;
 lock_profile_glb_q.elem_q = *q;
diff --git a/xen/common/version.c b/xen/common/version.c
index 223cb52..7414d2d 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -147,14 +147,14 @@ static int __init xen_build_init(void)
 int rc;
 
 /* --build-id invoked with wrong parameters. */
-if ( __note_gnu_build_id_end <= [0] )
+if ( SYMBOL(__note_gnu_build_id_end) <= [0] )
 return -ENODATA;
 
 /* Check for full Note header. */
-if ( [1] >= __note_gnu_build_id_end )
+if ( [1] >= SYMBOL(__note_gnu_build_id_end) )
 return -ENODATA;
 
-sz = (void *)__note_gnu_build_id_end - (void *)n;
+sz = (void *)SYMBOL(__note_gnu_build_id_end) - (void *)n;
 
 rc = 

[Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL

[Stefano Stabellini]

Introduce a macro, SYMBOL, which is similar to RELOC_HIDE, but it is
meant to be used everywhere symbols such as _stext and _etext are used
in the code. It can take an array type as a parameter, and it returns
the same type.

SYMBOL is needed when accessing symbols such as _stext and _etext
because the C standard forbids for both comparisons and substraction
(see C Standard, 6.5.6 [ISO/IEC 9899:2011] and [1]) between pointers
pointing to different objects. _stext, _etext, etc. are all pointers to
different objects from ANCI C point of view.

To work around potential C compiler issues (which have actually
been found, see the comment on top of RELOC_HIDE in Linux), and to help
with certifications, let's introduce some syntactic sugar to be used in
following patches.

[1] 
https://wiki.sei.cmu.edu/confluence/display/c/ARR36-C.+Do+not+subtract+or+compare+two+pointers+that+do+not+refer+to+the+same+array

Signed-off-by: Stefano Stabellini 
CC: jbeul...@suse.com
CC: andrew.coop...@citrix.com
CC: wei.l...@citrix.com
---
Changes in v6:
- drop acks
- don't use RELOC_HIDE for the implementation
- return native type from SYMBOL

Changes in v4:
- add acked-bys
- remove unneeded parenthesis

Changes in v3:
- improve commit message
- rename __symbol to SYMBOL to avoid name space violations

Changes in v2:
- do not cast return to char*
- move to common header
---
 xen/include/xen/compiler.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index ff6c0f5..d4c856c 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -99,6 +99,16 @@
 __asm__ ("" : "=r"(__ptr) : "0"(ptr));  \
 (typeof(ptr)) (__ptr + (off)); })
 
+/*
+ * Similar to RELOC_HIDE, but written to be used with symbols such as
+ * _stext and _etext to avoid undefined behavior comparing pointers to
+ * different objects. It can handle array types.
+ */
+#define SYMBOL(ptr)   \
+  ({ unsigned long __ptr;   \
+__asm__ ("" : "=r"(__ptr) : "0"(ptr));  \
+(typeof(*(ptr)) *) (__ptr); })
+
 #ifdef __GCC_ASM_FLAG_OUTPUTS__
 # define ASM_FLAG_OUT(yes, no) yes
 #else
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v6 2/4] xen/arm: use SYMBOL when required

[Stefano Stabellini]

Use SYMBOL in cases of comparisons and subtractions of:

_start, _end, __init_begin, __init_end, _stext, _etext,
__alt_instructions, __alt_instructions_end, __per_cpu_start,
__per_cpu_data_end, _splatform, _eplatform, _sdevice, _edevice,
_asdevice, _aedevice.

as by the C standard [1].

M3CM: Rule-18.2: Subtraction between pointers shall only be applied to
pointers that address elements of the same array

[1] 
https://wiki.sei.cmu.edu/confluence/display/c/ARR36-C.+Do+not+subtract+or+compare+two+pointers+that+do+not+refer+to+the+same+array

QAVerify: 2761
Signed-off-by: Stefano Stabellini 
CC: jbeul...@suse.com
CC: andrew.coop...@citrix.com
---
Changes in v6:
- more accurate commit message
- use new SYMBOL macro that returns the native type

Changes in v5:
- remove two spurious changes
- split into three patches
- remove SYMBOL() from derived variables

Changes in v4:
- only use SYMBOL where necessary, not "everywhere": comparisons and
  subtractions
- improve commit message
- remove some unnecessary casts
- fix some still unsafe casts
- extend checks to all symbols in xen/arch/x86/xen.lds.S and
  xen/arch/arm/xen.lds.S

Changes in v3:
- improve commit message
- no hard tabs
- rename __symbol to SYMBOL
- fix __end_vpci_array and __start_vpci_array
- avoid all comparisons between pointers: including (void *) casted
  returns from SYMBOL()
- remove useless casts to (unsigned long)

Changes in v2:
- cast return of SYMBOL to char* when required
- define __pa as unsigned long in is_kernel* functions
---
 xen/arch/arm/alternative.c|  4 ++--
 xen/arch/arm/arm32/livepatch.c|  2 +-
 xen/arch/arm/arm64/livepatch.c|  2 +-
 xen/arch/arm/device.c |  6 +++---
 xen/arch/arm/livepatch.c  |  4 ++--
 xen/arch/arm/mm.c | 13 +++--
 xen/arch/arm/percpu.c |  8 
 xen/arch/arm/platform.c   |  6 --
 xen/arch/arm/setup.c  |  6 --
 xen/include/asm-arm/grant_table.h |  3 ++-
 10 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
index 52ed7ed..ae738a9 100644
--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -188,7 +188,7 @@ static int __apply_alternatives_multi_stop(void *unused)
 int ret;
 struct alt_region region;
 mfn_t xen_mfn = virt_to_mfn(_start);
-paddr_t xen_size = _end - _start;
+paddr_t xen_size = SYMBOL(_end) - SYMBOL(_start);
 unsigned int xen_order = get_order_from_bytes(xen_size);
 void *xenmap;
 
@@ -206,7 +206,7 @@ static int __apply_alternatives_multi_stop(void *unused)
 region.begin = __alt_instructions;
 region.end = __alt_instructions_end;
 
-ret = __apply_alternatives(, xenmap - (void *)_start);
+ret = __apply_alternatives(, xenmap - (void *)SYMBOL(_start));
 /* The patching is not expected to fail during boot. */
 BUG_ON(ret != 0);
 
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index 41378a5..6bf9132 100644
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -56,7 +56,7 @@ void arch_livepatch_apply(struct livepatch_func *func)
 else
 insn = 0xe1a0; /* mov r0, r0 */
 
-new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+new_ptr = func->old_addr - (void *)SYMBOL(_start) + vmap_of_xen_text;
 len = len / sizeof(uint32_t);
 
 /* PATCH! */
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index 2247b92..ec49877 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -43,7 +43,7 @@ void arch_livepatch_apply(struct livepatch_func *func)
 /* Verified in livepatch_verify_distance. */
 ASSERT(insn != AARCH64_BREAK_FAULT);
 
-new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+new_ptr = func->old_addr - (void *)SYMBOL(_start) + vmap_of_xen_text;
 len = len / sizeof(uint32_t);
 
 /* PATCH! */
diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 70cd6c1..bb209be 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -35,7 +35,7 @@ int __init device_init(struct dt_device_node *dev, enum 
device_class class,
 if ( !dt_device_is_available(dev) || dt_device_for_passthrough(dev) )
 return  -ENODEV;
 
-for ( desc = _sdevice; desc != _edevice; desc++ )
+for ( desc = SYMBOL(_sdevice); desc != SYMBOL(_edevice); desc++ )
 {
 if ( desc->class != class )
 continue;
@@ -56,7 +56,7 @@ int __init acpi_device_init(enum device_class class, const 
void *data, int class
 {
 const struct acpi_device_desc *desc;
 
-for ( desc = _asdevice; desc != _aedevice; desc++ )
+for ( desc = SYMBOL(_asdevice); desc != SYMBOL(_aedevice); desc++ )
 {
 if ( ( desc->class != class ) || ( desc->class_type != class_type ) )
 continue;
@@ -75,7 +75,7 @@ enum device_class 

Re: [Xen-devel] [PATCH v3 04/15] argo: init, destroy and soft-reset, with enable command line opt

[Christopher Clark]

On Wed, Jan 9, 2019 at 6:16 AM Jason Andryuk  wrote:
> On Wed, Jan 9, 2019 at 1:48 AM Christopher Clark 
>  wrote:
> > On Tue, Jan 8, 2019 at 2:54 PM Jason Andryuk  wrote:
> > > On Mon, Jan 7, 2019 at 2:43 AM Christopher Clark 
> > >  wrote:
>
> > > > +
> > > > +/* A space-available notification that is awaiting sufficient space */
> > > > +struct pending_ent
> > > > +{
> > > > +/*
> > > > + * Pointer to the ring_info that this ent pertains to. Used to 
> > > > ensure that
> > > > + * ring_info->npending is decremented when ents for wildcard rings 
> > > > are
> > > > + * cancelled for domain destroy.
> > > > + * Caution: Must hold the correct locks before accessing ring_info 
> > > > via this.
> > >
> > > It would be clearer if this stated the correct locks.
> >
> > ok - it would mean duplicating the statement about which locks are
> > needed though, since it is explained elsewhere in the file, which means
> > it will need updating in two places if the locking requirements change.
> > That was why I worded it that way, as an indicator to go and find where
> > it is already described, to avoid that.
>
> "Caution" made me think *ring_info points from domain A's pending_ent
> to domain B's ring_info.  Reading patch 10 (notify op) I see that it
> really just points back to domain A's ring_info.  So the "Caution" is
> just that you still have to lock ring_info (L3) even though you can
> get to the pointer via L2.  Is that correct?

Yes, exactly.

> I agree a single location for the locking documentation is better than
> splitting or duplicating.  As long as no cross-domain locking is
> required, this is fine.

OK - thanks.

Christopher

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC v2 0/4] QEMU changes to do PVH boot

[Maran Wilson]

On 1/9/2019 11:53 AM, Boris Ostrovsky wrote:

On 1/9/19 6:53 AM, Stefano Garzarella wrote:

Hi Liam,

On Tue, Jan 8, 2019 at 3:47 PM Liam Merwick  wrote:

QEMU sets the hvm_modlist_entry in load_linux() after the call to
load_elfboot() and then qboot loads it in boot_pvh_from_fw_cfg()

But the current PVH patches don't handle initrd (they have
start_info.nr_modules == 1).

Looking in the linux kernel (arch/x86/platform/pvh/enlighten.c) I saw:
 /* The first module is always ramdisk. */
 if (pvh_start_info.nr_modules) {
 struct hvm_modlist_entry *modaddr =
 __va(pvh_start_info.modlist_paddr);
 pvh_bootparams.hdr.ramdisk_image = modaddr->paddr;
 pvh_bootparams.hdr.ramdisk_size = modaddr->size;
 }

So, putting start_info.nr_modules = 1, means that the first
hvm_modlist_entry should have the ramdisk paddr and size. Is it
correct?


That's my understanding.

I think what's missing, is that we just need Qemu or qboot/seabios to 
properly populate the pvh_start_info.modlist_paddr with the address (as 
usable by the guest) of the hvm_modlist_entry which correctly defines 
the details of the initrd that has already been loaded into memory that 
is accessible by the guest.


-Maran


During (or after) the call to load_elfboot() it looks like we'd need to
do something like what load_multiboot() does below (along with the
associated initialisation)

400 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
401 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
402 fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
403  sizeof(bootinfo));


In this case I think they used the FW_CFG_INITRD_* to pass bootinfo
varibales to the guest, maybe we could add something like what
linux_load() does:

 /* load initrd */
 if (initrd_filename) {
 ...
 initrd_addr = (initrd_max-initrd_size) & ~4095;

 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
 ...
 }

Then we can load the initrd in qboot or in the optionrom that I'm writing.

What do you think?


Why not specify this in pvh_start_info? This will be much faster for
everyone, no need to go through fw_cfg.

-boris




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [ovmf test] 131880: regressions - FAIL

[osstest service owner]

flight 131880 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131880/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i3866 xen-buildfail REGR. vs. 129475
 build-i386-xsm6 xen-buildfail REGR. vs. 129475
 build-amd64-xsm   6 xen-buildfail REGR. vs. 129475
 build-amd64   6 xen-buildfail REGR. vs. 129475

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-amd64-libvirt   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

version targeted for testing:
 ovmf a53a888de8f5fa8dbf75a381e28f25a5497572f1
baseline version:
 ovmf 5ae3184d8c59f7bbb84bad482df6b8020ba58188

Last test of basis   129475  2018-11-05 21:13:11 Z   64 days
Failing since129526  2018-11-06 20:49:26 Z   64 days  257 attempts
Testing same since   131796  2019-01-08 08:06:26 Z1 days6 attempts


People who touched revisions under test:
  Achin Gupta 
  Alex James 
  Ard Biesheuvel 
  Ashish Singhal 
  Bob Feng 
  bob.c.f...@intel.com 
  BobCF 
  Chasel Chiu 
  Chasel, Chiu 
  Chen A Chen 
  Chu, Maggie 
  Dandan Bi 
  David Wei 
  Derek Lin 
  Eric Dong 
  Feng, Bob C 
  Fu Siyuan 
  Gary Lin 
  Hao Wu 
  Jaben Carsey 
  Jagadeesh Ujja 
  Jeff Brasen 
  Jian J Wang 
  Jiaxin Wu 
  Jiewen Yao 
  Laszlo Ersek 
  Leif Lindholm 
  Liming Gao 
  Liu Yu 
  Maggie Chu 
  Marc Zyngier 
  Marcin Wojtas 
  Mike Maslenkin 
  Ming Huang 
  Pedroa Liu 
  Ruiyu Ni 
  Sami Mujawar 
  shenglei 
  Shenglei Zhang 
  Siyuan Fu 
  Star Zeng 
  Sughosh Ganu 
  Sumit Garg 
  Sun, Zailiang 
  Thomas Abraham 
  Thomas Rydman 
  Ting Ye 
  Tomasz Michalec 
  Vijayenthiran Subramaniam 
  Vladimir Olovyannikov 
  Wang BinX A 
  Wu Jiaxin 
  Ye Ting 
  Yonghong Zhu 
  yuchenlin 
  Zailiang Sun 
  Zhang, Chao B 
  Zhao, ZhiqiangX 
  Zhiju.Fan 
  zhijufan 
  ZhiqiangX Zhao 
  zwei4 

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.

(No revision log; it would be 5677 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Christopher Clark]

On Wed, Jan 9, 2019 at 10:28 AM Julien Grall  wrote:
>
>
>
> On Wed, 9 Jan 2019, 12:54 Wei Liu,  wrote:
>>
>> On Wed, Jan 09, 2019 at 12:02:34PM -0500, Julien Grall wrote:
>> > Hi,
>> >
>> > Sorry for the formatting. Sending it from my phone.
>> >
>> > On Wed, 9 Jan 2019, 11:03 Christopher Clark, 
>> > 
>> > wrote:
>> >
>> > > On Wed, Jan 9, 2019 at 7:56 AM Wei Liu  wrote:
>> > > >
>> > > > On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark wrote:
>> > > > > The register op is used by a domain to register a region of memory 
>> > > > > for
>> > > > > receiving messages from either a specified other domain, or, if
>> > > specifying a
>> > > > > wildcard, any domain.
>> > > > >
>> > > > > This operation creates a mapping within Xen's private address space
>> > > that
>> > > > > will remain resident for the lifetime of the ring. In subsequent
>> > > commits,
>> > > > > the hypervisor will use this mapping to copy data from a sending
>> > > domain into
>> > > > > this registered ring, making it accessible to the domain that
>> > > registered the
>> > > > > ring to receive data.
>> > > > >
>> > > > > Wildcard any-sender rings are default disabled and registration will 
>> > > > > be
>> > > > > refused with EPERM unless they have been specifically enabled with 
>> > > > > the
>> > > > > argo-mac boot option introduced here. The reason why the default for
>> > > > > wildcard rings is 'deny' is that there is currently no means to
>> > > protect the
>> > > > > ring from DoS by a noisy domain spamming the ring, affecting other
>> > > domains
>> > > > > ability to send to it. This will be addressed with XSM policy 
>> > > > > controls
>> > > in
>> > > > > subsequent work.
>> > > > >
>> > > > > Since denying access to any-sender rings is a significant functional
>> > > > > constraint, a new bootparam is provided to enable overriding this:
>> > > > >  "argo-mac" variable has allowed values: 'permissive' and 
>> > > > > 'enforcing'.
>> > > > > Even though this is a boolean variable, use these descriptive strings
>> > > in
>> > > > > order to make it obvious to an administrator that this has potential
>> > > > > security impact.
>> > > > >
>> > > > > The p2m type of the memory supplied by the guest for the ring must be
>> > > > > p2m_ram_rw and the memory will be pinned as PGT_writable_page while
>> > > the ring
>> > > > > is registered.
>> > > > >
>> > > > > xen_argo_page_descr_t type is introduced as a page descriptor, to
>> > > convey
>> > > > > both the physical address of the start of the page and its
>> > > granularity. The
>> > > > > smallest granularity page is assumed to be 4096 bytes and the lower
>> > > twelve
>> > > > > bits of the type are used to indicate the size of page of memory
>> > > supplied.
>> > > > > The implementation of the hypercall op currently only supports 4K
>> > > pages.
>> > > > >
>> > > >
>> > > > What is the resolution for the Arm issues mentioned by Julien? I read
>> > > > the conversation in previous thread. A solution seemed to have been
>> > > > agreed upon, but the changelog doesn't say anything about it.
>> > >
>> > > I made the interface changes that Julien had asked for. The register
>> > > op now takes arguments that can describe the granularitity of the
>> > > pages supplied, though only support for 4K pages is accepted in the
>> > > current implementation. I believe it meets Julien's requirements.
>> >
>> >
>> > I still don't think allowing 4K or 64K is the right solution to go. You are
>> > adding unnecessary burden in the hypervisor and would prevent optimization
>> > i the hypervisor and unwanted side effect.
>> >
>> > For instance a 64K hypervisor will always map 64K even when the guest is
>> > passing 4K. You also can't map everything contiguously in Xen (if you ever
>> > wanted to).
>> >
>> > We need to stick on a single chunk size. That could be different between
>> > Arm and x86. For Arm it would need to be 64KB.
>>
>> Doesn't enforcing 64KB granularity has its own limitation as well?
>> According to my understanding of arm (and this could be wrong), you
>> would need to have the guest allocate (via memory exchange perhaps) 64KB
>> machine contiguous memory even when the hypervisor doesn't need it to be
>> 64KB (when hypervisor is running on 4KB granularity).
>
>
> The 64K is just about the interface with the guest.
> The hypervisor could just split the 64K in 16 4K chunk. No need for memory 
> exchange here.
>
>>
>> I think having a method to return granularity to guest, like Stefano
>> suggested, is more sensible. Hypervisor will then reject registration
>> request which doesn't conform to the requirement.
>
>
> The problem is not that simple... For instance, 64K is required to support 
> 52-bits PA yet you may still want to run your current Debian on that platform.
>
> You can do that nicely on KVM but on Xen it is a pain due to the current 
> interface. If you use 4K you may end up to expose too much to the other side.
>
> The only viable solution here is 

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Christopher Clark]

On Wed, Jan 9, 2019 at 10:14 AM Julien Grall  wrote:
> On Wed, 9 Jan 2019, 12:18 Stefano Stabellini,  wrote:
>> On Wed, 9 Jan 2019, Julien Grall wrote:
>> > Hi,
>> > Sorry for the formatting. Sending it from my phone.
>> >
>> > On Wed, 9 Jan 2019, 11:03 Christopher Clark, 
>> >  wrote:
>> >   On Wed, Jan 9, 2019 at 7:56 AM Wei Liu  wrote:
>> >   >
>> >   > On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark wrote:
>> >   > > The register op is used by a domain to register a region of 
>> > memory for
>> >   > > receiving messages from either a specified other domain, or, if 
>> > specifying a
>> >   > > wildcard, any domain.
>> >   > >
>> >   > > xen_argo_page_descr_t type is introduced as a page descriptor, 
>> > to convey
>> >   > > both the physical address of the start of the page and its 
>> > granularity. The
>> >   > > smallest granularity page is assumed to be 4096 bytes and the 
>> > lower twelve
>> >   > > bits of the type are used to indicate the size of page of memory 
>> > supplied.
>> >   > > The implementation of the hypercall op currently only supports 
>> > 4K pages.
>> >   >
>> >   > What is the resolution for the Arm issues mentioned by Julien? I 
>> > read
>> >   > the conversation in previous thread. A solution seemed to have been
>> >   > agreed upon, but the changelog doesn't say anything about it.
>> >
>> >   I made the interface changes that Julien had asked for. The register
>> >   op now takes arguments that can describe the granularitity of the
>> >   pages supplied, though only support for 4K pages is accepted in the
>> >   current implementation. I believe it meets Julien's requirements.
>> >
>> > I still don't think allowing 4K or 64K is the right solution to go. You 
>> > are adding unnecessary burden in the hypervisor and would
>> > prevent optimization i the hypervisor and unwanted side effect.
>> >
>> > For instance a 64K hypervisor will always map 64K even when the guest is 
>> > passing 4K. You also can't map everything contiguously
>> > in Xen (if you ever wanted to).
>> >
>> > We need to stick on a single chunk size. That could be different between 
>> > Arm and x86. For Arm it would need to be 64KB.
>>
>> I don't think we should force 64K as the only granularity on ARM. It
>> causes unnecessary overhead and confusion on 4K-only deployments that
>> are almost all our use-cases today.
>
> Why a confusion? People should read the documentation when writing a driver...
>
>> One option is to make the granularity configurable at the guest side,
>> like Christopher did, letting the guest specifying the granularity. The
>> hypervisor could return -ENOSYS if the specified granularity is not
>> supported.
>>
>> The other option is having the hypervisor export the granularity it
>> supports for this interface: Xen would say "I support only 4K".
>> Tomorrow, it could change and Xen could say "I support only 64K". Then,
>> it would be up to the guest passing a page of the right granularity to
>> the hypervisor. I think this is probably the best option, but it would
>> require the addition of one hypercall to retrieve the supported
>> granularity from Xen.
>
> I would recommend to read my initial answers on the first series to 
> understand why allowing 4K is an issue.
>
> AFAIK virtio and UEFI has restrictions to allow a guest running agnostically 
> of the hypervisor page-granularity. An example is to mandate 64K chunk or 
> 64KB aligned address.
>
> With your suggestion you are going to break many use-cases if the hypervisor 
> is moving to 64KB. At worst it could introduce security issues. At best 
> preventing optimization in the hypervisor or prevent guest running (bad for 
> backward compatibility).
>
> Actually, this is not going to help moving towards 64K in Argo because you 
> would still have to modify the kernel. So this does not meet my requirements.
>
> I don't think requiring 64K chunk is going to be a major issue nor a concern. 
> Unless you expect small ring... Christoffer, what is the expected size?

The current implementation of the Linux driver has a default ring size
of 128K and I would expect that to be a common case. I'm not familiar
with any current use cases where smaller than 64K would be likely, but
I can imagine it could potentially be useful to have the option to do
so in a memory-constrained embedded environment running a service that
handles large numbers of short-lived connections, so running frequent
setup and teardown of many rings, with only small amounts of data
exchanged on each. That's not my current use case though, so it is
speculating a bit.

> Another solution was to require contiguous guest physical memory. That would 
> solve quite a few problem on Arm. But Christoffer had some convincing point 
> to not implement this.
>
> As I said before, I know this is not going to be the only place with that 
> issue. I merely wanted to start tackling the 

Re: [Xen-devel] [RFC v2 0/4] QEMU changes to do PVH boot

[Boris Ostrovsky]

On 1/9/19 6:53 AM, Stefano Garzarella wrote:
> Hi Liam,
>
> On Tue, Jan 8, 2019 at 3:47 PM Liam Merwick  wrote:
>> QEMU sets the hvm_modlist_entry in load_linux() after the call to
>> load_elfboot() and then qboot loads it in boot_pvh_from_fw_cfg()
>>
>> But the current PVH patches don't handle initrd (they have
>> start_info.nr_modules == 1).
> Looking in the linux kernel (arch/x86/platform/pvh/enlighten.c) I saw:
> /* The first module is always ramdisk. */
> if (pvh_start_info.nr_modules) {
> struct hvm_modlist_entry *modaddr =
> __va(pvh_start_info.modlist_paddr);
> pvh_bootparams.hdr.ramdisk_image = modaddr->paddr;
> pvh_bootparams.hdr.ramdisk_size = modaddr->size;
> }
>
> So, putting start_info.nr_modules = 1, means that the first
> hvm_modlist_entry should have the ramdisk paddr and size. Is it
> correct?
>
>
>> During (or after) the call to load_elfboot() it looks like we'd need to
>> do something like what load_multiboot() does below (along with the
>> associated initialisation)
>>
>> 400 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
>> 401 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
>> 402 fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
>> 403  sizeof(bootinfo));
>>
> In this case I think they used the FW_CFG_INITRD_* to pass bootinfo
> varibales to the guest, maybe we could add something like what
> linux_load() does:
>
> /* load initrd */
> if (initrd_filename) {
> ...
> initrd_addr = (initrd_max-initrd_size) & ~4095;
>
> fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
> fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
> fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, 
> initrd_size);
> ...
> }
>
> Then we can load the initrd in qboot or in the optionrom that I'm writing.
>
> What do you think?


Why not specify this in pvh_start_info? This will be much faster for
everyone, no need to go through fw_cfg.

-boris


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 09/15] argo: implement the sendv op; evtchn: expose send_guest_global_virq

[Roger Pau Monné]

 to.On Mon, Jan 7, 2019 at 8:44 AM Christopher Clark
 wrote:
>
> sendv operation is invoked to perform a synchronous send of buffers
> contained in iovs to a remote domain's registered ring.
>
> It takes:
>  * A destination address (domid, port) for the ring to send to.
>It performs a most-specific match lookup, to allow for wildcard.
>  * A source address, used to inform the destination of where to reply.
>  * The address of an array of iovs containing the data to send
>  * .. and the length of that array of iovs
>  * and a 32-bit message type, available to communicate message context
>data (eg. kernel-to-kernel, separate from the application data).
>
> If insufficient space exists in the destination ring, it will return
> -EAGAIN and Xen will notify the caller when sufficient space becomes
> available.
>
> Accesses to the ring indices are appropriately atomic. The rings are
> mapped into Xen's private address space to write as needed and the
> mappings are retained for later use.
>
> Fixed-size types are used in some areas within this code where caution
> around avoiding integer overflow is important.
>
> Notifications are sent to guests via VIRQ and send_guest_global_virq is
> exposed in the change to enable argo to call it. VIRQ_ARGO_MESSAGE is
> claimed from the VIRQ previously reserved for this purpose (#11).
>
> The VIRQ notification method is used rather than sending events using
> evtchn functions directly because:
>
> * no current event channel type is an exact fit for the intended
>   behaviour. ECS_IPI is closest, but it disallows migration to
>   other VCPUs which is not necessarily a requirement for Argo.
>
> * at the point of argo_init, allocation of an event channel is
>   complicated by none of the guest VCPUs being initialized yet
>   and the event channel logic expects that a valid event channel
>   has a present VCPU.
>
> * at the point of signalling a notification, the VIRQ logic is already
>   defensive: if d->vcpu[0] is NULL, the notification is just silently
>   dropped, whereas the evtchn_send logic is not so defensive: vcpu[0]
>   must not be NULL, otherwise a null pointer dereference occurs.
>
> Using a VIRQ removes the need for the guest to query to determine which
> event channel notifications will be delivered on. This is also likely to
> simplify establishing future L0/L1 nested hypervisor argo communication.
>
> Signed-off-by: Christopher Clark 
> ---
> The previous double-read of iovs from guest memory has been removed.
>
> v2 self: use ring_info backpointer in pending_ent to maintain npending
> v2 feedback Jan: drop cookie, implement teardown
> v2 self: pending_queue: reap stale ents when in need of space
> v2 self: pending_requeue: reclaim ents for stale domains
> v2.feedback Jan: only override sender domid if DOMID_ANY
> v2 feedback Jan: drop message from argo_message_op
> v2 self: check npending vs maximum limit
> v2 self: get_sanitized_ring instead of get_rx_ptr
> v2 feedback v1#13 Jan: remove double read from ringbuf insert, lower MAX_IOV
> v2 self: make iov_count const
> v2 self: iov_count : return EMSGSIZE for message too big
> v2 self: OVERHAUL
> v2 self: s/argo_pending_ent/pending_ent/g
> v2 feedback v1#13 Roger: use OS-supplied roundup; drop from public header
> v1,2 feedback Jan/Roger/Paul: drop errno returning guest access functions
> v1 feedback Roger, Jan: drop argo prefix on static functions
> v1 feedback #13 Jan: drop guest_handle_okay when using copy_from_guest
> - reorder do_argo_op logic
> v2 self: add _hnd suffix to iovs variable name to indicate guest handle type
> v2 self: replace use of XEN_GUEST_HANDLE_NULL with two existing macros
>
> v1 #15 feedback, Jan: sendv op : s/ECONNREFUSED/ESRCH/
> v1 #5 (#15) feedback Paul: sendv: use currd in do_argo_message_op
> v1 #13 (#15) feedback Paul: sendv op: do/while reindent only
> v1 #13 (#15) feedback Paul: sendv op: do/while: argo_ringbuf_insert to goto 
> style
> v1 #13 (#15) feedback Paul: sendv op: do/while: reindent only again
> v1 #13 (#15) feedback Paul: sendv op: do/while : goto
> v1 #15 feedback Paul: sendv op: make page var: unsigned
> v1 #15 feedback Paul: sendv op: new local var for PAGE_SIZE - offset
> v1 #8 feedback Jan: XEN_GUEST_HANDLE : C89 compliance
> v1 rebase after switching register op from pfns to page descriptors
> v1 self: move iov DEFINE_XEN_GUEST_HANDLE out of public header into argo.c
> v1 #13 (#15) feedback Paul: fix loglevel for guest-triggered messages
> v1 : add compat xlat.lst entries
> v1 self: switched notification to send_guest_global_virq instead of event
> v1: fix gprintk use for ARM as its defn dislikes split format strings
> v1: init len variable to satisfy ARM compiler initialized checking
> v1 #13 feedback Jan: rename page var
> v1:#14 feedback Jan: uint8_t* -> void*
> v1: #13 feedback Jan: public namespace: prefix with xen
> v1: #13 feedback Jan: blank line after case op in do_argo_message_op
> v1: #15 feedback Jan: add comments explaining why the writes 

[Xen-devel] [linux-linus test] 131834: regressions - FAIL

[osstest service owner]

flight 131834 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131834/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-amd64-pvgrub  7 xen-bootfail REGR. vs. 125898
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
125898
 test-amd64-amd64-xl-qcow2 7 xen-boot fail REGR. vs. 125898
 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 125898
 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 125898
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 125898
 test-amd64-amd64-libvirt-vhd  7 xen-boot fail REGR. vs. 125898
 test-amd64-amd64-xl-multivcpu  7 xen-bootfail REGR. vs. 125898
 test-amd64-amd64-pygrub   7 xen-boot fail REGR. vs. 125898
 test-amd64-amd64-libvirt-xsm  7 xen-boot fail REGR. vs. 125898
 test-amd64-amd64-rumprun-amd64  7 xen-boot   fail REGR. vs. 125898
 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host   fail REGR. vs. 125898
 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host   fail REGR. vs. 125898
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 125898
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-rumprun-i386  7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 125898
 test-amd64-amd64-xl-pvhv2-intel  7 xen-boot  fail REGR. vs. 125898
 test-amd64-i386-libvirt-xsm   7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 125898
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot  fail REGR. vs. 125898
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 125898
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 125898
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 125898
 test-amd64-amd64-examine  8 reboot   fail REGR. vs. 125898
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail REGR. vs. 
125898

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  7 xen-boot fail REGR. vs. 125898

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-credit1   7 xen-bootfail baseline untested
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125898
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125898
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125898
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 125898
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125898
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125898
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125898
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 125898
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-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-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-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-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-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 

Re: [Xen-devel] [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Wed, Jan 09, 2019 at 09:03:25AM -0700, Tamas K Lengyel wrote:
> Ah, thanks for that info! We should probably put that on the wiki too ;)

Done:
https://wiki.xenproject.org/index.php?title=OVMF=19075=19074

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Julien Grall]

On Wed, 9 Jan 2019, 12:54 Wei Liu,  wrote:

> On Wed, Jan 09, 2019 at 12:02:34PM -0500, Julien Grall wrote:
> > Hi,
> >
> > Sorry for the formatting. Sending it from my phone.
> >
> > On Wed, 9 Jan 2019, 11:03 Christopher Clark, <
> christopher.w.cl...@gmail.com>
> > wrote:
> >
> > > On Wed, Jan 9, 2019 at 7:56 AM Wei Liu  wrote:
> > > >
> > > > On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark wrote:
> > > > > The register op is used by a domain to register a region of memory
> for
> > > > > receiving messages from either a specified other domain, or, if
> > > specifying a
> > > > > wildcard, any domain.
> > > > >
> > > > > This operation creates a mapping within Xen's private address space
> > > that
> > > > > will remain resident for the lifetime of the ring. In subsequent
> > > commits,
> > > > > the hypervisor will use this mapping to copy data from a sending
> > > domain into
> > > > > this registered ring, making it accessible to the domain that
> > > registered the
> > > > > ring to receive data.
> > > > >
> > > > > Wildcard any-sender rings are default disabled and registration
> will be
> > > > > refused with EPERM unless they have been specifically enabled with
> the
> > > > > argo-mac boot option introduced here. The reason why the default
> for
> > > > > wildcard rings is 'deny' is that there is currently no means to
> > > protect the
> > > > > ring from DoS by a noisy domain spamming the ring, affecting other
> > > domains
> > > > > ability to send to it. This will be addressed with XSM policy
> controls
> > > in
> > > > > subsequent work.
> > > > >
> > > > > Since denying access to any-sender rings is a significant
> functional
> > > > > constraint, a new bootparam is provided to enable overriding this:
> > > > >  "argo-mac" variable has allowed values: 'permissive' and
> 'enforcing'.
> > > > > Even though this is a boolean variable, use these descriptive
> strings
> > > in
> > > > > order to make it obvious to an administrator that this has
> potential
> > > > > security impact.
> > > > >
> > > > > The p2m type of the memory supplied by the guest for the ring must
> be
> > > > > p2m_ram_rw and the memory will be pinned as PGT_writable_page while
> > > the ring
> > > > > is registered.
> > > > >
> > > > > xen_argo_page_descr_t type is introduced as a page descriptor, to
> > > convey
> > > > > both the physical address of the start of the page and its
> > > granularity. The
> > > > > smallest granularity page is assumed to be 4096 bytes and the lower
> > > twelve
> > > > > bits of the type are used to indicate the size of page of memory
> > > supplied.
> > > > > The implementation of the hypercall op currently only supports 4K
> > > pages.
> > > > >
> > > >
> > > > What is the resolution for the Arm issues mentioned by Julien? I read
> > > > the conversation in previous thread. A solution seemed to have been
> > > > agreed upon, but the changelog doesn't say anything about it.
> > >
> > > I made the interface changes that Julien had asked for. The register
> > > op now takes arguments that can describe the granularitity of the
> > > pages supplied, though only support for 4K pages is accepted in the
> > > current implementation. I believe it meets Julien's requirements.
> >
> >
> > I still don't think allowing 4K or 64K is the right solution to go. You
> are
> > adding unnecessary burden in the hypervisor and would prevent
> optimization
> > i the hypervisor and unwanted side effect.
> >
> > For instance a 64K hypervisor will always map 64K even when the guest is
> > passing 4K. You also can't map everything contiguously in Xen (if you
> ever
> > wanted to).
> >
> > We need to stick on a single chunk size. That could be different between
> > Arm and x86. For Arm it would need to be 64KB.
>
> Doesn't enforcing 64KB granularity has its own limitation as well?
> According to my understanding of arm (and this could be wrong), you
> would need to have the guest allocate (via memory exchange perhaps) 64KB
> machine contiguous memory even when the hypervisor doesn't need it to be
> 64KB (when hypervisor is running on 4KB granularity).


The 64K is just about the interface with the guest.
The hypervisor could just split the 64K in 16 4K chunk. No need for memory
exchange here.


> I think having a method to return granularity to guest, like Stefano
> suggested, is more sensible. Hypervisor will then reject registration
> request which doesn't conform to the requirement.
>

The problem is not that simple... For instance, 64K is required to support
52-bits PA yet you may still want to run your current Debian on that
platform.

You can do that nicely on KVM but on Xen it is a pain due to the current
interface. If you use 4K you may end up to expose too much to the other
side.

The only viable solution here is a full re-design of the ABI for Arm. We
can do that step by step or at one go.

The discussion here was to start solving it on Argo so that's one less step
to do. Christoffer kindly try 

Re: [Xen-devel] [PATCH] tmem: default to off

[Konrad Rzeszutek Wilk]

On Wed, Jan 09, 2019 at 05:16:17PM +, Wei Liu wrote:
> On Wed, Jan 09, 2019 at 02:05:19AM -0700, Jan Beulich wrote:
> > As a short term alternative to deleting the code, default its building
> > to off (overridable in EXPERT mode only). Additionally make sure other
> > related baggage (LZO code) won't be carried when the option is off (with
> > TMEM scheduled to be deleted anyway, I didn't want to introduce a
> > separate Kconfig option to control the LZO compression code, and hence
> > CONFIG_TMEM is used directly there). Similarly I couldn't be bothered to
> > add actual content to the command line option doc for the two affected
> > options.
> > 
> > Signed-off-by: Jan Beulich 
> 
> Acked-by: Wei Liu 


Acked-by: Konrad Rzeszutek Wilk 

Thank you!

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Julien Grall]

Ki

On Wed, 9 Jan 2019, 12:18 Stefano Stabellini, 
wrote:

> On Wed, 9 Jan 2019, Julien Grall wrote:
> > Hi,
> > Sorry for the formatting. Sending it from my phone.
> >
> > On Wed, 9 Jan 2019, 11:03 Christopher Clark, <
> christopher.w.cl...@gmail.com> wrote:
> >   On Wed, Jan 9, 2019 at 7:56 AM Wei Liu 
> wrote:
> >   >
> >   > On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark
> wrote:
> >   > > The register op is used by a domain to register a region of
> memory for
> >   > > receiving messages from either a specified other domain, or,
> if specifying a
> >   > > wildcard, any domain.
> >   > >
> >   > > This operation creates a mapping within Xen's private address
> space that
> >   > > will remain resident for the lifetime of the ring. In
> subsequent commits,
> >   > > the hypervisor will use this mapping to copy data from a
> sending domain into
> >   > > this registered ring, making it accessible to the domain that
> registered the
> >   > > ring to receive data.
> >   > >
> >   > > Wildcard any-sender rings are default disabled and
> registration will be
> >   > > refused with EPERM unless they have been specifically enabled
> with the
> >   > > argo-mac boot option introduced here. The reason why the
> default for
> >   > > wildcard rings is 'deny' is that there is currently no means
> to protect the
> >   > > ring from DoS by a noisy domain spamming the ring, affecting
> other domains
> >   > > ability to send to it. This will be addressed with XSM policy
> controls in
> >   > > subsequent work.
> >   > >
> >   > > Since denying access to any-sender rings is a significant
> functional
> >   > > constraint, a new bootparam is provided to enable overriding
> this:
> >   > >  "argo-mac" variable has allowed values: 'permissive' and
> 'enforcing'.
> >   > > Even though this is a boolean variable, use these descriptive
> strings in
> >   > > order to make it obvious to an administrator that this has
> potential
> >   > > security impact.
> >   > >
> >   > > The p2m type of the memory supplied by the guest for the ring
> must be
> >   > > p2m_ram_rw and the memory will be pinned as PGT_writable_page
> while the ring
> >   > > is registered.
> >   > >
> >   > > xen_argo_page_descr_t type is introduced as a page descriptor,
> to convey
> >   > > both the physical address of the start of the page and its
> granularity. The
> >   > > smallest granularity page is assumed to be 4096 bytes and the
> lower twelve
> >   > > bits of the type are used to indicate the size of page of
> memory supplied.
> >   > > The implementation of the hypercall op currently only supports
> 4K pages.
> >   > >
> >   >
> >   > What is the resolution for the Arm issues mentioned by Julien? I
> read
> >   > the conversation in previous thread. A solution seemed to have
> been
> >   > agreed upon, but the changelog doesn't say anything about it.
> >
> >   I made the interface changes that Julien had asked for. The
> register
> >   op now takes arguments that can describe the granularitity of the
> >   pages supplied, though only support for 4K pages is accepted in the
> >   current implementation. I believe it meets Julien's requirements.
> >
> >
> > I still don't think allowing 4K or 64K is the right solution to go. You
> are adding unnecessary burden in the hypervisor and would
> > prevent optimization i the hypervisor and unwanted side effect.
> >
> > For instance a 64K hypervisor will always map 64K even when the guest is
> passing 4K. You also can't map everything contiguously
> > in Xen (if you ever wanted to).
> >
> > We need to stick on a single chunk size. That could be different between
> Arm and x86. For Arm it would need to be 64KB.
>
> Hi Julien!
>
> I don't think we should force 64K as the only granularity on ARM. It
> causes unnecessary overhead and confusion on 4K-only deployments that
> are almost all our use-cases today.


Why a confusion? People should read the documentation when writing a
driver...


> One option is to make the granularity configurable at the guest side,
> like Christopher did, letting the guest specifying the granularity. The
> hypervisor could return -ENOSYS if the specified granularity is not
> supported.
>
> The other option is having the hypervisor export the granularity it
> supports for this interface: Xen would say "I support only 4K".
> Tomorrow, it could change and Xen could say "I support only 64K". Then,
> it would be up to the guest passing a page of the right granularity to
> the hypervisor. I think this is probably the best option, but it would
> require the addition of one hypercall to retrieve the supported
> granularity from Xen.


I would recommend to read my initial answers on the first series to
understand why allowing 4K is an issue.

AFAIK virtio and UEFI has restrictions to allow a 

Re: [Xen-devel] [PATCH v3 09/15] argo: implement the sendv op; evtchn: expose send_guest_global_virq

[Jason Andryuk]

On Mon, Jan 7, 2019 at 2:43 AM Christopher Clark
 wrote:



> @@ -342,6 +357,413 @@ update_tx_ptr(struct argo_ring_info *ring_info, 
> uint32_t tx_ptr)
>  smp_wmb();
>  }
>
> +static int
> +memcpy_to_guest_ring(struct argo_ring_info *ring_info, uint32_t offset,
> + const void *src, XEN_GUEST_HANDLE(uint8_t) src_hnd,
> + uint32_t len)
> +{
> +unsigned int mfns_index = offset >> PAGE_SHIFT;
> +void *dst;
> +int ret;
> +unsigned int src_offset = 0;
> +
> +ASSERT(spin_is_locked(_info->lock));
> +
> +offset &= ~PAGE_MASK;
> +
> +if ( (len > XEN_ARGO_MAX_RING_SIZE) || (offset > XEN_ARGO_MAX_RING_SIZE) 
> )
> +return -EFAULT;
> +
> +while ( (offset + len) > PAGE_SIZE )
> +{
> +unsigned int head_len = PAGE_SIZE - offset;

I think this while loop could be re-written as
while (len) {
head_len = len > PAGE_SIZE ? PAGE_SIZE - offset: len;

and then the extra copying below outside the loop could be dropped.

The first loop does a partial copy at offset and then sets offset=0.
The next N loops copy exactly PAGE_SIZE.
The Final copy does the remaining len bytes.

> +
> +ret = ring_map_page(ring_info, mfns_index, );
> +if ( ret )
> +return ret;
> +
> +if ( src )
> +{
> +memcpy(dst + offset, src + src_offset, head_len);
> +src_offset += head_len;
> +}
> +else
> +{
> +ret = copy_from_guest(dst + offset, src_hnd, head_len) ?
> +-EFAULT : 0;
> +if ( ret )
> +return ret;
> +
> +guest_handle_add_offset(src_hnd, head_len);
> +}
> +
> +mfns_index++;
> +len -= head_len;
> +offset = 0;
> +}
> +
> +ret = ring_map_page(ring_info, mfns_index, );
> +if ( ret )
> +{
> +argo_dprintk("argo: ring (vm%u:%x vm%d) %p attempted to map page"
> + " %d of %d\n", ring_info->id.domain_id, 
> ring_info->id.port,
> + ring_info->id.partner_id, ring_info, mfns_index,
> + ring_info->nmfns);
> +return ret;
> +}
> +
> +if ( src )
> +memcpy(dst + offset, src + src_offset, len);
> +else
> +ret = copy_from_guest(dst + offset, src_hnd, len) ? -EFAULT : 0;
> +
> +return ret;
> +}



> +
> +/*
> + * iov_count returns its count on success via an out variable to avoid
> + * potential for a negative return value to be used incorrectly
> + * (eg. coerced into an unsigned variable resulting in a large incorrect 
> value)
> + */
> +static int
> +iov_count(const xen_argo_iov_t *piov, unsigned long niov, uint32_t *count)
> +{
> +uint32_t sum_iov_lens = 0;
> +
> +if ( niov > XEN_ARGO_MAXIOV )
> +return -EINVAL;
> +
> +while ( niov-- )
> +{
> +/* valid iovs must have the padding field set to zero */
> +if ( piov->pad )
> +{
> +argo_dprintk("invalid iov: padding is not zero\n");
> +return -EINVAL;
> +}
> +
> +/* check each to protect sum against integer overflow */
> +if ( piov->iov_len > XEN_ARGO_MAX_RING_SIZE )

Should this be MAX_ARGO_MESSAGE_SIZE?  MAX_ARGO_MESSAGE_SIZE is less
than XEN_ARGO_MAX_RING_SIZE, so we can pass this check and then just
fail the one below.

> +{
> +argo_dprintk("invalid iov_len: too big (%u)>%llu\n",
> + piov->iov_len, XEN_ARGO_MAX_RING_SIZE);
> +return -EINVAL;
> +}
> +
> +sum_iov_lens += piov->iov_len;
> +
> +/*
> + * Again protect sum from integer overflow
> + * and ensure total msg size will be within bounds.
> + */
> +if ( sum_iov_lens > MAX_ARGO_MESSAGE_SIZE )
> +{
> +argo_dprintk("invalid iov series: total message too big\n");
> +return -EMSGSIZE;
> +}
> +
> +piov++;
> +}
> +
> +*count = sum_iov_lens;
> +
> +return 0;
> +}
> +



> @@ -1073,6 +1683,49 @@ do_argo_op(unsigned int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg1,
>  break;
>  }
>
> +case XEN_ARGO_OP_sendv:
> +{
> +xen_argo_send_addr_t send_addr;
> +
> +XEN_GUEST_HANDLE_PARAM(xen_argo_send_addr_t) send_addr_hnd =
> +guest_handle_cast(arg1, xen_argo_send_addr_t);
> +XEN_GUEST_HANDLE_PARAM(xen_argo_iov_t) iovs_hnd =
> +guest_handle_cast(arg2, xen_argo_iov_t);
> +/* arg3 is niov */
> +/* arg4 is message_type. Must be a 32-bit value. */
> +
> +rc = copy_from_guest(_addr, send_addr_hnd, 1) ? -EFAULT : 0;
> +if ( rc )
> +break;
> +
> +if ( send_addr.src.domain_id == XEN_ARGO_DOMID_ANY )
> +send_addr.src.domain_id = currd->domain_id;
> +
> +/* No domain is currently authorized to send on behalf of another */
> +if ( unlikely(send_addr.src.domain_id != 

[Xen-devel] [freebsd-master test] 131876: regressions - FAIL

[osstest service owner]

flight 131876 freebsd-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131876/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xen-freebsd   5 host-install(5)  fail REGR. vs. 131783
 build-amd64-freebsd-again 5 host-install(5)  fail REGR. vs. 131783

version targeted for testing:
 freebsd  87ee73823609e3a2a692bc475424b75a234e5d05
baseline version:
 freebsd  2a52bc55467e95f92e1024cd558df3930df99594

Last test of basis   131783  2019-01-07 09:19:04 Z2 days
Testing same since   131876  2019-01-09 09:19:19 Z0 days1 attempts


People who touched revisions under test:
  0mp <0...@freebsd.org>
  cem 
  chuck 
  cperciva 
  cy 
  delphij 
  emaste 
  glebius 
  ian 
  imp 
  jkim 
  kevans 
  kib 
  markj 
  pfg 
  pjd 
  shurd 
  tuexen 

jobs:
 build-amd64-freebsd-againfail
 build-amd64-freebsd  pass
 build-amd64-xen-freebsd  fail



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.

(No revision log; it would be 369 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Wei Liu]

On Wed, Jan 09, 2019 at 12:02:34PM -0500, Julien Grall wrote:
> Hi,
> 
> Sorry for the formatting. Sending it from my phone.
> 
> On Wed, 9 Jan 2019, 11:03 Christopher Clark, 
> wrote:
> 
> > On Wed, Jan 9, 2019 at 7:56 AM Wei Liu  wrote:
> > >
> > > On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark wrote:
> > > > The register op is used by a domain to register a region of memory for
> > > > receiving messages from either a specified other domain, or, if
> > specifying a
> > > > wildcard, any domain.
> > > >
> > > > This operation creates a mapping within Xen's private address space
> > that
> > > > will remain resident for the lifetime of the ring. In subsequent
> > commits,
> > > > the hypervisor will use this mapping to copy data from a sending
> > domain into
> > > > this registered ring, making it accessible to the domain that
> > registered the
> > > > ring to receive data.
> > > >
> > > > Wildcard any-sender rings are default disabled and registration will be
> > > > refused with EPERM unless they have been specifically enabled with the
> > > > argo-mac boot option introduced here. The reason why the default for
> > > > wildcard rings is 'deny' is that there is currently no means to
> > protect the
> > > > ring from DoS by a noisy domain spamming the ring, affecting other
> > domains
> > > > ability to send to it. This will be addressed with XSM policy controls
> > in
> > > > subsequent work.
> > > >
> > > > Since denying access to any-sender rings is a significant functional
> > > > constraint, a new bootparam is provided to enable overriding this:
> > > >  "argo-mac" variable has allowed values: 'permissive' and 'enforcing'.
> > > > Even though this is a boolean variable, use these descriptive strings
> > in
> > > > order to make it obvious to an administrator that this has potential
> > > > security impact.
> > > >
> > > > The p2m type of the memory supplied by the guest for the ring must be
> > > > p2m_ram_rw and the memory will be pinned as PGT_writable_page while
> > the ring
> > > > is registered.
> > > >
> > > > xen_argo_page_descr_t type is introduced as a page descriptor, to
> > convey
> > > > both the physical address of the start of the page and its
> > granularity. The
> > > > smallest granularity page is assumed to be 4096 bytes and the lower
> > twelve
> > > > bits of the type are used to indicate the size of page of memory
> > supplied.
> > > > The implementation of the hypercall op currently only supports 4K
> > pages.
> > > >
> > >
> > > What is the resolution for the Arm issues mentioned by Julien? I read
> > > the conversation in previous thread. A solution seemed to have been
> > > agreed upon, but the changelog doesn't say anything about it.
> >
> > I made the interface changes that Julien had asked for. The register
> > op now takes arguments that can describe the granularitity of the
> > pages supplied, though only support for 4K pages is accepted in the
> > current implementation. I believe it meets Julien's requirements.
> 
> 
> I still don't think allowing 4K or 64K is the right solution to go. You are
> adding unnecessary burden in the hypervisor and would prevent optimization
> i the hypervisor and unwanted side effect.
> 
> For instance a 64K hypervisor will always map 64K even when the guest is
> passing 4K. You also can't map everything contiguously in Xen (if you ever
> wanted to).
> 
> We need to stick on a single chunk size. That could be different between
> Arm and x86. For Arm it would need to be 64KB.

Doesn't enforcing 64KB granularity has its own limitation as well?
According to my understanding of arm (and this could be wrong), you
would need to have the guest allocate (via memory exchange perhaps) 64KB
machine contiguous memory even when the hypervisor doesn't need it to be
64KB (when hypervisor is running on 4KB granularity).

I think having a method to return granularity to guest, like Stefano
suggested, is more sensible. Hypervisor will then reject registration
request which doesn't conform to the requirement.

Wei.

> 
> Cheers,
> 
> 
> > thanks,
> >
> > Christopher
> >
> > ___
> > 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

[Xen-devel] [libvirt test] 131857: tolerable all pass - PUSHED

[osstest service owner]

flight 131857 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131857/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 131766
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 131766
 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 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-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  45b439c3af000eb41c819068d093406810dd036c
baseline version:
 libvirt  99c33a7cbf7fdb607a43a91342a44a7a22e20bfb

Last test of basis   131766  2019-01-06 04:18:57 Z3 days
Failing since131792  2019-01-08 04:19:08 Z1 days2 attempts
Testing same since   131857  2019-01-09 04:18:51 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Eric Blake 
  Erik Skultety 
  Han Han 
  Ján Tomko 
  Michal Privoznik 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-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-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd 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/libvirt.git
   99c33a7cbf..45b439c3af  45b439c3af000eb41c819068d093406810dd036c -> 
xen-tested-master

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Stefano Stabellini]

On Wed, 9 Jan 2019, Julien Grall wrote:
> Hi,
> Sorry for the formatting. Sending it from my phone.
> 
> On Wed, 9 Jan 2019, 11:03 Christopher Clark,  
> wrote:
>   On Wed, Jan 9, 2019 at 7:56 AM Wei Liu  wrote:
>   >
>   > On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark wrote:
>   > > The register op is used by a domain to register a region of memory 
> for
>   > > receiving messages from either a specified other domain, or, if 
> specifying a
>   > > wildcard, any domain.
>   > >
>   > > This operation creates a mapping within Xen's private address space 
> that
>   > > will remain resident for the lifetime of the ring. In subsequent 
> commits,
>   > > the hypervisor will use this mapping to copy data from a sending 
> domain into
>   > > this registered ring, making it accessible to the domain that 
> registered the
>   > > ring to receive data.
>   > >
>   > > Wildcard any-sender rings are default disabled and registration 
> will be
>   > > refused with EPERM unless they have been specifically enabled with 
> the
>   > > argo-mac boot option introduced here. The reason why the default for
>   > > wildcard rings is 'deny' is that there is currently no means to 
> protect the
>   > > ring from DoS by a noisy domain spamming the ring, affecting other 
> domains
>   > > ability to send to it. This will be addressed with XSM policy 
> controls in
>   > > subsequent work.
>   > >
>   > > Since denying access to any-sender rings is a significant functional
>   > > constraint, a new bootparam is provided to enable overriding this:
>   > >  "argo-mac" variable has allowed values: 'permissive' and 
> 'enforcing'.
>   > > Even though this is a boolean variable, use these descriptive 
> strings in
>   > > order to make it obvious to an administrator that this has potential
>   > > security impact.
>   > >
>   > > The p2m type of the memory supplied by the guest for the ring must 
> be
>   > > p2m_ram_rw and the memory will be pinned as PGT_writable_page while 
> the ring
>   > > is registered.
>   > >
>   > > xen_argo_page_descr_t type is introduced as a page descriptor, to 
> convey
>   > > both the physical address of the start of the page and its 
> granularity. The
>   > > smallest granularity page is assumed to be 4096 bytes and the lower 
> twelve
>   > > bits of the type are used to indicate the size of page of memory 
> supplied.
>   > > The implementation of the hypercall op currently only supports 4K 
> pages.
>   > >
>   >
>   > What is the resolution for the Arm issues mentioned by Julien? I read
>   > the conversation in previous thread. A solution seemed to have been
>   > agreed upon, but the changelog doesn't say anything about it.
> 
>   I made the interface changes that Julien had asked for. The register
>   op now takes arguments that can describe the granularitity of the
>   pages supplied, though only support for 4K pages is accepted in the
>   current implementation. I believe it meets Julien's requirements.
> 
> 
> I still don't think allowing 4K or 64K is the right solution to go. You are 
> adding unnecessary burden in the hypervisor and would
> prevent optimization i the hypervisor and unwanted side effect.
> 
> For instance a 64K hypervisor will always map 64K even when the guest is 
> passing 4K. You also can't map everything contiguously
> in Xen (if you ever wanted to).
> 
> We need to stick on a single chunk size. That could be different between Arm 
> and x86. For Arm it would need to be 64KB.

Hi Julien!

I don't think we should force 64K as the only granularity on ARM. It
causes unnecessary overhead and confusion on 4K-only deployments that
are almost all our use-cases today.

One option is to make the granularity configurable at the guest side,
like Christopher did, letting the guest specifying the granularity. The
hypervisor could return -ENOSYS if the specified granularity is not
supported.

The other option is having the hypervisor export the granularity it
supports for this interface: Xen would say "I support only 4K".
Tomorrow, it could change and Xen could say "I support only 64K". Then,
it would be up to the guest passing a page of the right granularity to
the hypervisor. I think this is probably the best option, but it would
require the addition of one hypercall to retrieve the supported
granularity from Xen.___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tmem: default to off

[Wei Liu]

On Wed, Jan 09, 2019 at 02:05:19AM -0700, Jan Beulich wrote:
> As a short term alternative to deleting the code, default its building
> to off (overridable in EXPERT mode only). Additionally make sure other
> related baggage (LZO code) won't be carried when the option is off (with
> TMEM scheduled to be deleted anyway, I didn't want to introduce a
> separate Kconfig option to control the LZO compression code, and hence
> CONFIG_TMEM is used directly there). Similarly I couldn't be bothered to
> add actual content to the command line option doc for the two affected
> options.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 13/25] argo: implement the register op

[Razvan Cojocaru]

On 1/9/19 6:50 PM, Tamas K Lengyel wrote:

On Wed, Jan 9, 2019 at 9:48 AM Razvan Cojocaru
 wrote:


On 1/9/19 6:34 PM, Roger Pau Monné wrote:

Maybe this is use-case is different, but how does introspection handle
accesses to the shared info page or the runstate info for example?

I would consider argo to be the same in this regard.


Not exactly: The shared info page is special in any event. For
runstate info (and alike - there's also struct vcpu_time_info)
I'd question correctness of the current handling. If that's
wrong already, I'd prefer if the issue wasn't spread.


There are also grants, which when used together with another guest on
the same host could allow to bypass introspection AFAICT? (unless
there's some policy applied that limit grant sharing to trusted
domains)

TBH I'm not sure how to handle hypoervisor accesses with
introspection.  My knowledge of introspection is fairly limited, but
it pauses the guest and sends a notification to an in guest agent. I'm
not sure this is applicable to hypervisor writes, since it's not
possible to pause hypervisor execution and wait for a response from a
guest agent.



Introspection applications only care about memory accesses performed
by the guest. Hypervisor accesses to monitored pages are not included
when monitoring - it is actually a feature when using the emulator in
Xen to continue guest execution because the hypervisor ignores EPT
memory permissions that trip the guest for introspection. So having
the hypervisor access memory or a grant-shared page being accessed in
another domain are not a problem for introspection.


Can't then two guests running on the same host be able to completely
bypass introspection? I guess you prevent this by limiting to which
guests pages can be shared?


Would these two guests be HVM guests? Introspection only works for HVM
guests. I'm not sure I follow your scenario though. How would these
guests collaborate to escape introspection via grants?


If there are two domains acting maliciously in concert to bypass
monitoring of memory writes they could achieve that with grants, yes.
Say a write-monitored page is grant-shared to another domain, which
then does the write on behalf of the first. I wouldn't say that's
"completely bypassing introspection" though, there are many types of
events that can be monitored, write-accesses are only one. I'm not
aware of any mechanism that can be used to limit which pages can be
shared but you can use XSM to restrict which domains can share pages
to begin with. That's normally enough.


Right, I agree. We're not currently dealing with that case and assume 
that XSM (or a similar mechanism) will be used in scenarios where this 
level of access is possible.



Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] Fwd: [PATCH 13/25] argo: implement the register op

[Roger Pau Monné]

On Wed, Jan 9, 2019 at 5:51 PM Tamas K Lengyel  wrote:
>
> On Wed, Jan 9, 2019 at 9:48 AM Razvan Cojocaru
>  wrote:
> >
> > On 1/9/19 6:34 PM, Roger Pau Monné wrote:
> > > Maybe this is use-case is different, but how does introspection handle
> > > accesses to the shared info page or the runstate info for example?
> > >
> > > I would consider argo to be the same in this regard.
> > 
> >  Not exactly: The shared info page is special in any event. For
> >  runstate info (and alike - there's also struct vcpu_time_info)
> >  I'd question correctness of the current handling. If that's
> >  wrong already, I'd prefer if the issue wasn't spread.
> > >>>
> > >>> There are also grants, which when used together with another guest on
> > >>> the same host could allow to bypass introspection AFAICT? (unless
> > >>> there's some policy applied that limit grant sharing to trusted
> > >>> domains)
> > >>>
> > >>> TBH I'm not sure how to handle hypoervisor accesses with
> > >>> introspection.  My knowledge of introspection is fairly limited, but
> > >>> it pauses the guest and sends a notification to an in guest agent. I'm
> > >>> not sure this is applicable to hypervisor writes, since it's not
> > >>> possible to pause hypervisor execution and wait for a response from a
> > >>> guest agent.
> > >>>
> > >>
> > >> Introspection applications only care about memory accesses performed
> > >> by the guest. Hypervisor accesses to monitored pages are not included
> > >> when monitoring - it is actually a feature when using the emulator in
> > >> Xen to continue guest execution because the hypervisor ignores EPT
> > >> memory permissions that trip the guest for introspection. So having
> > >> the hypervisor access memory or a grant-shared page being accessed in
> > >> another domain are not a problem for introspection.
> > >
> > > Can't then two guests running on the same host be able to completely
> > > bypass introspection? I guess you prevent this by limiting to which
> > > guests pages can be shared?
> >
> > Would these two guests be HVM guests? Introspection only works for HVM
> > guests. I'm not sure I follow your scenario though. How would these
> > guests collaborate to escape introspection via grants?
>
> If there are two domains acting maliciously in concert to bypass
> monitoring of memory writes they could achieve that with grants, yes.
> Say a write-monitored page is grant-shared to another domain, which
> then does the write on behalf of the first. I wouldn't say that's
> "completely bypassing introspection" though, there are many types of
> events that can be monitored, write-accesses are only one. I'm not
> aware of any mechanism that can be used to limit which pages can be
> shared but you can use XSM to restrict which domains can share pages
> to begin with. That's normally enough.

Yes, I assumed that would be the way to protect against such attacks,
ie: limiting to which guests pages can be shared. I think just making
sure the right access checks are placed in XSM (just like they are for
grants) should be enough.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Julien Grall]

Hi,

Sorry for the formatting. Sending it from my phone.

On Wed, 9 Jan 2019, 11:03 Christopher Clark, 
wrote:

> On Wed, Jan 9, 2019 at 7:56 AM Wei Liu  wrote:
> >
> > On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark wrote:
> > > The register op is used by a domain to register a region of memory for
> > > receiving messages from either a specified other domain, or, if
> specifying a
> > > wildcard, any domain.
> > >
> > > This operation creates a mapping within Xen's private address space
> that
> > > will remain resident for the lifetime of the ring. In subsequent
> commits,
> > > the hypervisor will use this mapping to copy data from a sending
> domain into
> > > this registered ring, making it accessible to the domain that
> registered the
> > > ring to receive data.
> > >
> > > Wildcard any-sender rings are default disabled and registration will be
> > > refused with EPERM unless they have been specifically enabled with the
> > > argo-mac boot option introduced here. The reason why the default for
> > > wildcard rings is 'deny' is that there is currently no means to
> protect the
> > > ring from DoS by a noisy domain spamming the ring, affecting other
> domains
> > > ability to send to it. This will be addressed with XSM policy controls
> in
> > > subsequent work.
> > >
> > > Since denying access to any-sender rings is a significant functional
> > > constraint, a new bootparam is provided to enable overriding this:
> > >  "argo-mac" variable has allowed values: 'permissive' and 'enforcing'.
> > > Even though this is a boolean variable, use these descriptive strings
> in
> > > order to make it obvious to an administrator that this has potential
> > > security impact.
> > >
> > > The p2m type of the memory supplied by the guest for the ring must be
> > > p2m_ram_rw and the memory will be pinned as PGT_writable_page while
> the ring
> > > is registered.
> > >
> > > xen_argo_page_descr_t type is introduced as a page descriptor, to
> convey
> > > both the physical address of the start of the page and its
> granularity. The
> > > smallest granularity page is assumed to be 4096 bytes and the lower
> twelve
> > > bits of the type are used to indicate the size of page of memory
> supplied.
> > > The implementation of the hypercall op currently only supports 4K
> pages.
> > >
> >
> > What is the resolution for the Arm issues mentioned by Julien? I read
> > the conversation in previous thread. A solution seemed to have been
> > agreed upon, but the changelog doesn't say anything about it.
>
> I made the interface changes that Julien had asked for. The register
> op now takes arguments that can describe the granularitity of the
> pages supplied, though only support for 4K pages is accepted in the
> current implementation. I believe it meets Julien's requirements.


I still don't think allowing 4K or 64K is the right solution to go. You are
adding unnecessary burden in the hypervisor and would prevent optimization
i the hypervisor and unwanted side effect.

For instance a 64K hypervisor will always map 64K even when the guest is
passing 4K. You also can't map everything contiguously in Xen (if you ever
wanted to).

We need to stick on a single chunk size. That could be different between
Arm and x86. For Arm it would need to be 64KB.

Cheers,


> thanks,
>
> Christopher
>
> ___
> 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 13/25] argo: implement the register op

[Roger Pau Monné]

On Wed, Jan 9, 2019 at 5:51 PM Tamas K Lengyel  wrote:
>
> On Wed, Jan 9, 2019 at 9:48 AM Razvan Cojocaru
>  wrote:
> >
> > On 1/9/19 6:34 PM, Roger Pau Monné wrote:
> > > Maybe this is use-case is different, but how does introspection handle
> > > accesses to the shared info page or the runstate info for example?
> > >
> > > I would consider argo to be the same in this regard.
> > 
> >  Not exactly: The shared info page is special in any event. For
> >  runstate info (and alike - there's also struct vcpu_time_info)
> >  I'd question correctness of the current handling. If that's
> >  wrong already, I'd prefer if the issue wasn't spread.
> > >>>
> > >>> There are also grants, which when used together with another guest on
> > >>> the same host could allow to bypass introspection AFAICT? (unless
> > >>> there's some policy applied that limit grant sharing to trusted
> > >>> domains)
> > >>>
> > >>> TBH I'm not sure how to handle hypoervisor accesses with
> > >>> introspection.  My knowledge of introspection is fairly limited, but
> > >>> it pauses the guest and sends a notification to an in guest agent. I'm
> > >>> not sure this is applicable to hypervisor writes, since it's not
> > >>> possible to pause hypervisor execution and wait for a response from a
> > >>> guest agent.
> > >>>
> > >>
> > >> Introspection applications only care about memory accesses performed
> > >> by the guest. Hypervisor accesses to monitored pages are not included
> > >> when monitoring - it is actually a feature when using the emulator in
> > >> Xen to continue guest execution because the hypervisor ignores EPT
> > >> memory permissions that trip the guest for introspection. So having
> > >> the hypervisor access memory or a grant-shared page being accessed in
> > >> another domain are not a problem for introspection.
> > >
> > > Can't then two guests running on the same host be able to completely
> > > bypass introspection? I guess you prevent this by limiting to which
> > > guests pages can be shared?
> >
> > Would these two guests be HVM guests? Introspection only works for HVM
> > guests. I'm not sure I follow your scenario though. How would these
> > guests collaborate to escape introspection via grants?
>
> If there are two domains acting maliciously in concert to bypass
> monitoring of memory writes they could achieve that with grants, yes.
> Say a write-monitored page is grant-shared to another domain, which
> then does the write on behalf of the first. I wouldn't say that's
> "completely bypassing introspection" though, there are many types of
> events that can be monitored, write-accesses are only one. I'm not
> aware of any mechanism that can be used to limit which pages can be
> shared but you can use XSM to restrict which domains can share pages
> to begin with. That's normally enough.

Yes, I assumed that would be the way to protect against such attacks,
ie: limiting to which guests pages can be shared. I think just making
sure the right access checks are placed in XSM (just like they are for
grants) should be enough.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tmem: default to off

[Roger Pau Monné]

On Wed, Jan 9, 2019 at 4:28 PM Jan Beulich  wrote:
>
> >>> On 09.01.19 at 16:02,  wrote:
> > From: Xen-devel  on behalf of Jan 
> > Beulich
> > :
> > --- a/xen/common/Makefile
> > +++ b/xen/common/Makefile
> > @@ -21,7 +21,7 @@ obj-$(CONFIG_KEXEC) += kimage.o
> >  obj-y += lib.o
> >  obj-$(CONFIG_NEEDS_LIST_SORT) += list_sort.o
> >  obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
> > -obj-y += lzo.o
> > +obj-$(CONFIG_TMEM) += lzo.o
> >
> > Here you completely disable the build of lzo if tmem is not enabled.
> >
> >  obj-$(CONFIG_MEM_ACCESS) += mem_access.o
> >  obj-y += memory.o
> >  obj-y += monitor.o
> > @@ -66,8 +66,9 @@ obj-bin-y += warning.init.o
> >  obj-$(CONFIG_XENOPROF) += xenoprof.o
> >  obj-y += xmalloc_tlsf.o
> >
> > -obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo
> > unlz4 earlycpio,$(n).init.o)
> > -
> > +lzo-y := lzo
> > +lzo-$(CONFIG_TMEM) :=
> > +obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma 
> > $(lzo-y) unlzo unlz4 earlycpio,$(n).init.o)
> >
> > Here however you always build unlzo.c, which AFAICT makes use of the
> > lzo1x_decompress_safe function that's defined in lzo.c.
>
> Note the (new) definition and use of lzo-y, so together with unlzo.init.o
> lzo.init.o will also be built. (I can assure you that I did test with TMEM
> enabled and disabled.)

Oh sorry, missed it. That makes the lzo parts need when not using tmem
be placed (and checked) to be in the init section.

Reviewed-by: Roger Pau Monné 

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 13/25] argo: implement the register op

[Tamas K Lengyel]

On Wed, Jan 9, 2019 at 9:48 AM Razvan Cojocaru
 wrote:
>
> On 1/9/19 6:34 PM, Roger Pau Monné wrote:
> > Maybe this is use-case is different, but how does introspection handle
> > accesses to the shared info page or the runstate info for example?
> >
> > I would consider argo to be the same in this regard.
> 
>  Not exactly: The shared info page is special in any event. For
>  runstate info (and alike - there's also struct vcpu_time_info)
>  I'd question correctness of the current handling. If that's
>  wrong already, I'd prefer if the issue wasn't spread.
> >>>
> >>> There are also grants, which when used together with another guest on
> >>> the same host could allow to bypass introspection AFAICT? (unless
> >>> there's some policy applied that limit grant sharing to trusted
> >>> domains)
> >>>
> >>> TBH I'm not sure how to handle hypoervisor accesses with
> >>> introspection.  My knowledge of introspection is fairly limited, but
> >>> it pauses the guest and sends a notification to an in guest agent. I'm
> >>> not sure this is applicable to hypervisor writes, since it's not
> >>> possible to pause hypervisor execution and wait for a response from a
> >>> guest agent.
> >>>
> >>
> >> Introspection applications only care about memory accesses performed
> >> by the guest. Hypervisor accesses to monitored pages are not included
> >> when monitoring - it is actually a feature when using the emulator in
> >> Xen to continue guest execution because the hypervisor ignores EPT
> >> memory permissions that trip the guest for introspection. So having
> >> the hypervisor access memory or a grant-shared page being accessed in
> >> another domain are not a problem for introspection.
> >
> > Can't then two guests running on the same host be able to completely
> > bypass introspection? I guess you prevent this by limiting to which
> > guests pages can be shared?
>
> Would these two guests be HVM guests? Introspection only works for HVM
> guests. I'm not sure I follow your scenario though. How would these
> guests collaborate to escape introspection via grants?

If there are two domains acting maliciously in concert to bypass
monitoring of memory writes they could achieve that with grants, yes.
Say a write-monitored page is grant-shared to another domain, which
then does the write on behalf of the first. I wouldn't say that's
"completely bypassing introspection" though, there are many types of
events that can be monitored, write-accesses are only one. I'm not
aware of any mechanism that can be used to limit which pages can be
shared but you can use XSM to restrict which domains can share pages
to begin with. That's normally enough.

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 13/25] argo: implement the register op

[Razvan Cojocaru]

On 1/9/19 6:34 PM, Roger Pau Monné wrote:

Maybe this is use-case is different, but how does introspection handle
accesses to the shared info page or the runstate info for example?

I would consider argo to be the same in this regard.


Not exactly: The shared info page is special in any event. For
runstate info (and alike - there's also struct vcpu_time_info)
I'd question correctness of the current handling. If that's
wrong already, I'd prefer if the issue wasn't spread.


There are also grants, which when used together with another guest on
the same host could allow to bypass introspection AFAICT? (unless
there's some policy applied that limit grant sharing to trusted
domains)

TBH I'm not sure how to handle hypoervisor accesses with
introspection.  My knowledge of introspection is fairly limited, but
it pauses the guest and sends a notification to an in guest agent. I'm
not sure this is applicable to hypervisor writes, since it's not
possible to pause hypervisor execution and wait for a response from a
guest agent.



Introspection applications only care about memory accesses performed
by the guest. Hypervisor accesses to monitored pages are not included
when monitoring - it is actually a feature when using the emulator in
Xen to continue guest execution because the hypervisor ignores EPT
memory permissions that trip the guest for introspection. So having
the hypervisor access memory or a grant-shared page being accessed in
another domain are not a problem for introspection.


Can't then two guests running on the same host be able to completely
bypass introspection? I guess you prevent this by limiting to which
guests pages can be shared?


Would these two guests be HVM guests? Introspection only works for HVM 
guests. I'm not sure I follow your scenario though. How would these 
guests collaborate to escape introspection via grants?



If that's the case, and introspection doesn't care about hypervisor
accesses to guest pages, then just getting a reference to the
underlying page when the ring is setup should be enough. There's no
need to check the gfn -> mfn relation every time there's an hypervisor
access to the ring.


I think so, but I might be missing something.


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [xen-unstable-smoke test] 131881: tolerable all pass - PUSHED

[osstest service owner]

flight 131881 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131881/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 13 migrate-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  808cff4c2af66afd61973451aeb7e708732abf90
baseline version:
 xen  f35a59452d5f5da20bef8acdc25f837ac782071c

Last test of basis   131879  2019-01-09 12:00:34 Z0 days
Testing same since   131881  2019-01-09 15:00:46 Z0 days1 attempts


People who touched revisions under test:
  Dario Faggioli 
  Juergen Gross 
  Sergey Dyasli 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 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
   f35a59452d..808cff4c2a  808cff4c2af66afd61973451aeb7e708732abf90 -> smoke

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 13/25] argo: implement the register op

[Roger Pau Monné]

On Wed, Jan 9, 2019 at 5:17 PM Tamas K Lengyel  wrote:
>
> On Mon, Jan 7, 2019 at 2:01 AM Roger Pau Monné  wrote:
> >
> > Adding the introspection guys.
> >
> > On Fri, Jan 04, 2019 at 08:47:04AM -0700, Jan Beulich wrote:
> > > >>> On 04.01.19 at 16:35,  wrote:
> > > > On Fri, Jan 04, 2019 at 06:22:19AM -0700, Jan Beulich wrote:
> > > >> >>> On 04.01.19 at 09:57,  wrote:
> > > >> > On Fri, Dec 21, 2018 at 03:05:03PM -0800, Christopher Clark wrote:
> > > >> >> On Thu, Dec 20, 2018 at 4:52 AM Roger Pau Monné 
> > > >> >>  wrote:
> > > >> >> >
> > > >> >> > On Wed, Dec 19, 2018 at 09:41:59PM -0800, Christopher Clark wrote:
> > > >> >> > > On Wed, Dec 12, 2018 at 8:48 AM Roger Pau Monné 
> > > >> >> > > 
> > > > wrote:
> > > >> >> > > >
> > > >> >> > > > On Fri, Nov 30, 2018 at 05:32:52PM -0800, Christopher Clark 
> > > >> >> > > > wrote:
> > > >> >> > Then I wonder why you need such check in any case if the code can
> > > >> >> > handle such cases, the more than the check itself is racy.
> > > >> >>
> > > >> >> OK, so at the root of the question here is: does it matter what the 
> > > >> >> p2m
> > > >> >> type of the memory is at these points:
> > > >> >>
> > > >> >> 1) when the gfn is translated to mfn, at the time of ring 
> > > >> >> registration
> > > >> >
> > > >> > This is the important check, because that's where you should take a
> > > >> > reference to the page. In this case you should check that the page is
> > > >> > of ram_rw type.
> > > >> >
> > > >> >> 2) when the hypervisor writes into guest memory:
> > > >> >> - where the tx_ptr index is initialized in the register op
> > > >> >> - where ringbuf data is written in sendv
> > > >> >> - where ring description data is written in notify
> > > >> >
> > > >> > As long as you keep a reference to the pages that are part of the 
> > > >> > ring
> > > >> > you don't need to do any checks when writing/reading from them. If 
> > > >> > the
> > > >> > guest messes up it's p2m and does change the gfn -> mfn mappings for
> > > >> > pages that are part of the ring that's the guest problem, the
> > > >> > hypervisor still has a reference to those pages so they won't be
> > > >> > reused.
> > > >>
> > > >> For use cases like introspection this may not be fully correct,
> > > >> but it may also be that my understanding there isn't fully
> > > >> correct. If introspection agents care about _any_ writes to
> > > >> a page, hypervisor ones (which in most cases are merely
> > > >> writes on behalf of the guest) might matter as well. I think
> > > >> to decide whether page accesses need to be accompanied
> > > >> by any checks (and if so, which ones) one needs to
> > > >> - establish what p2m type transitions are possible for a
> > > >>   given page,
> > > >> - verify what restrictions may occur "behind the back" of
> > > >>   the entity wanting to do the accesses,
> > > >> - explore whether doing the extra checking at p2m type
> > > >>   change time wouldn't be better than at the time of access.
> > > >
> > > > Maybe this is use-case is different, but how does introspection handle
> > > > accesses to the shared info page or the runstate info for example?
> > > >
> > > > I would consider argo to be the same in this regard.
> > >
> > > Not exactly: The shared info page is special in any event. For
> > > runstate info (and alike - there's also struct vcpu_time_info)
> > > I'd question correctness of the current handling. If that's
> > > wrong already, I'd prefer if the issue wasn't spread.
> >
> > There are also grants, which when used together with another guest on
> > the same host could allow to bypass introspection AFAICT? (unless
> > there's some policy applied that limit grant sharing to trusted
> > domains)
> >
> > TBH I'm not sure how to handle hypoervisor accesses with
> > introspection.  My knowledge of introspection is fairly limited, but
> > it pauses the guest and sends a notification to an in guest agent. I'm
> > not sure this is applicable to hypervisor writes, since it's not
> > possible to pause hypervisor execution and wait for a response from a
> > guest agent.
> >
>
> Introspection applications only care about memory accesses performed
> by the guest. Hypervisor accesses to monitored pages are not included
> when monitoring - it is actually a feature when using the emulator in
> Xen to continue guest execution because the hypervisor ignores EPT
> memory permissions that trip the guest for introspection. So having
> the hypervisor access memory or a grant-shared page being accessed in
> another domain are not a problem for introspection.

Can't then two guests running on the same host be able to completely
bypass introspection? I guess you prevent this by limiting to which
guests pages can be shared?

If that's the case, and introspection doesn't care about hypervisor
accesses to guest pages, then just getting a reference to the
underlying page when the ring is setup should be enough. There's no
need to check the gfn -> mfn relation 

Re: [Xen-devel] [PATCH 13/25] argo: implement the register op

[Razvan Cojocaru]

On 1/9/19 6:15 PM, Tamas K Lengyel wrote:

On Mon, Jan 7, 2019 at 2:01 AM Roger Pau Monné  wrote:


Adding the introspection guys.

On Fri, Jan 04, 2019 at 08:47:04AM -0700, Jan Beulich wrote:

On 04.01.19 at 16:35,  wrote:

On Fri, Jan 04, 2019 at 06:22:19AM -0700, Jan Beulich wrote:

On 04.01.19 at 09:57,  wrote:

On Fri, Dec 21, 2018 at 03:05:03PM -0800, Christopher Clark wrote:

On Thu, Dec 20, 2018 at 4:52 AM Roger Pau Monné  wrote:


On Wed, Dec 19, 2018 at 09:41:59PM -0800, Christopher Clark wrote:

On Wed, Dec 12, 2018 at 8:48 AM Roger Pau Monné 

wrote:


On Fri, Nov 30, 2018 at 05:32:52PM -0800, Christopher Clark wrote:

Then I wonder why you need such check in any case if the code can
handle such cases, the more than the check itself is racy.


OK, so at the root of the question here is: does it matter what the p2m
type of the memory is at these points:

1) when the gfn is translated to mfn, at the time of ring registration


This is the important check, because that's where you should take a
reference to the page. In this case you should check that the page is
of ram_rw type.


2) when the hypervisor writes into guest memory:
 - where the tx_ptr index is initialized in the register op
 - where ringbuf data is written in sendv
 - where ring description data is written in notify


As long as you keep a reference to the pages that are part of the ring
you don't need to do any checks when writing/reading from them. If the
guest messes up it's p2m and does change the gfn -> mfn mappings for
pages that are part of the ring that's the guest problem, the
hypervisor still has a reference to those pages so they won't be
reused.


For use cases like introspection this may not be fully correct,
but it may also be that my understanding there isn't fully
correct. If introspection agents care about _any_ writes to
a page, hypervisor ones (which in most cases are merely
writes on behalf of the guest) might matter as well. I think
to decide whether page accesses need to be accompanied
by any checks (and if so, which ones) one needs to
- establish what p2m type transitions are possible for a
   given page,
- verify what restrictions may occur "behind the back" of
   the entity wanting to do the accesses,
- explore whether doing the extra checking at p2m type
   change time wouldn't be better than at the time of access.


Maybe this is use-case is different, but how does introspection handle
accesses to the shared info page or the runstate info for example?

I would consider argo to be the same in this regard.


Not exactly: The shared info page is special in any event. For
runstate info (and alike - there's also struct vcpu_time_info)
I'd question correctness of the current handling. If that's
wrong already, I'd prefer if the issue wasn't spread.


There are also grants, which when used together with another guest on
the same host could allow to bypass introspection AFAICT? (unless
there's some policy applied that limit grant sharing to trusted
domains)

TBH I'm not sure how to handle hypoervisor accesses with
introspection.  My knowledge of introspection is fairly limited, but
it pauses the guest and sends a notification to an in guest agent. I'm
not sure this is applicable to hypervisor writes, since it's not
possible to pause hypervisor execution and wait for a response from a
guest agent.



Introspection applications only care about memory accesses performed
by the guest. Hypervisor accesses to monitored pages are not included
when monitoring - it is actually a feature when using the emulator in
Xen to continue guest execution because the hypervisor ignores EPT
memory permissions that trip the guest for introspection. So having
the hypervisor access memory or a grant-shared page being accessed in
another domain are not a problem for introspection.


Indeed, that's how it goes.


Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC PATCH 4/6] vm_event: Use slotted channels for sync requests.

[Razvan Cojocaru]

On 12/20/18 4:28 PM, Paul Durrant wrote:

-Original Message-
From: Petre Ovidiu PIRCALABU [mailto:ppircal...@bitdefender.com]
Sent: 20 December 2018 14:26
To: Paul Durrant ; xen-devel@lists.xenproject.org
Cc: Stefano Stabellini ; Wei Liu
; Razvan Cojocaru ; Konrad
Rzeszutek Wilk ; George Dunlap
; Andrew Cooper ; Ian
Jackson ; Tim (Xen.org) ; Julien
Grall ; Tamas K Lengyel ; Jan
Beulich ; Roger Pau Monne 
Subject: Re: [Xen-devel] [RFC PATCH 4/6] vm_event: Use slotted channels
for sync requests.

On Thu, 2018-12-20 at 12:05 +, Paul Durrant wrote:

The memory for the asynchronous ring and the synchronous channels
will
be allocated from domheap and mapped to the controlling domain
using the
foreignmemory_map_resource interface. Unlike the current
implementation,
the allocated pages are not part of the target DomU, so they will
not be
reclaimed when the vm_event domain is disabled.


Why re-invent the wheel here? The ioreq infrastructure already does
pretty much everything you need AFAICT.

   Paul


I wanted preseve as much as possible from the existing vm_event DOMCTL
interface and add only the necessary code to allocate and map the
vm_event_pages.


That means we have two subsystems duplicating a lot of functionality though. It 
would be much better to use ioreq server if possible than provide a 
compatibility interface via DOMCTL.


Just to clarify the compatibility issue: there's a third element between 
Xen and the introspection application, the Linux kernel which needs to 
be fairly recent for the whole ioreq machinery to work. The quemu code 
also seems to fallback to the old way of working if that's the case.


This means that there's a choice to be made here: either we keep 
backwards compatibility with the old vm_event interface (in which case 
we can't drop the waitqueue code), or we switch to the new one and leave 
older setups in the dust (but there's less code duplication and we can 
get rid of the waitqueue code).


In any event, it's not very clear (to me, at least) how the envisioned 
ioreq replacement should work. I assume we're meant to use the whole 
infrastructure (as opposed to what we're now doing, which is to only use 
the map-hypervisor-memory part), i.e. both mapping and signaling. Could 
we discuss this in more detail? Are there any docs on this or ioreq 
minimal clients (like xen-access.c is for vm_event) we might use?



Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 13/25] argo: implement the register op

[Tamas K Lengyel]

On Mon, Jan 7, 2019 at 2:01 AM Roger Pau Monné  wrote:
>
> Adding the introspection guys.
>
> On Fri, Jan 04, 2019 at 08:47:04AM -0700, Jan Beulich wrote:
> > >>> On 04.01.19 at 16:35,  wrote:
> > > On Fri, Jan 04, 2019 at 06:22:19AM -0700, Jan Beulich wrote:
> > >> >>> On 04.01.19 at 09:57,  wrote:
> > >> > On Fri, Dec 21, 2018 at 03:05:03PM -0800, Christopher Clark wrote:
> > >> >> On Thu, Dec 20, 2018 at 4:52 AM Roger Pau Monné 
> > >> >>  wrote:
> > >> >> >
> > >> >> > On Wed, Dec 19, 2018 at 09:41:59PM -0800, Christopher Clark wrote:
> > >> >> > > On Wed, Dec 12, 2018 at 8:48 AM Roger Pau Monné 
> > >> >> > > 
> > > wrote:
> > >> >> > > >
> > >> >> > > > On Fri, Nov 30, 2018 at 05:32:52PM -0800, Christopher Clark 
> > >> >> > > > wrote:
> > >> >> > Then I wonder why you need such check in any case if the code can
> > >> >> > handle such cases, the more than the check itself is racy.
> > >> >>
> > >> >> OK, so at the root of the question here is: does it matter what the 
> > >> >> p2m
> > >> >> type of the memory is at these points:
> > >> >>
> > >> >> 1) when the gfn is translated to mfn, at the time of ring registration
> > >> >
> > >> > This is the important check, because that's where you should take a
> > >> > reference to the page. In this case you should check that the page is
> > >> > of ram_rw type.
> > >> >
> > >> >> 2) when the hypervisor writes into guest memory:
> > >> >> - where the tx_ptr index is initialized in the register op
> > >> >> - where ringbuf data is written in sendv
> > >> >> - where ring description data is written in notify
> > >> >
> > >> > As long as you keep a reference to the pages that are part of the ring
> > >> > you don't need to do any checks when writing/reading from them. If the
> > >> > guest messes up it's p2m and does change the gfn -> mfn mappings for
> > >> > pages that are part of the ring that's the guest problem, the
> > >> > hypervisor still has a reference to those pages so they won't be
> > >> > reused.
> > >>
> > >> For use cases like introspection this may not be fully correct,
> > >> but it may also be that my understanding there isn't fully
> > >> correct. If introspection agents care about _any_ writes to
> > >> a page, hypervisor ones (which in most cases are merely
> > >> writes on behalf of the guest) might matter as well. I think
> > >> to decide whether page accesses need to be accompanied
> > >> by any checks (and if so, which ones) one needs to
> > >> - establish what p2m type transitions are possible for a
> > >>   given page,
> > >> - verify what restrictions may occur "behind the back" of
> > >>   the entity wanting to do the accesses,
> > >> - explore whether doing the extra checking at p2m type
> > >>   change time wouldn't be better than at the time of access.
> > >
> > > Maybe this is use-case is different, but how does introspection handle
> > > accesses to the shared info page or the runstate info for example?
> > >
> > > I would consider argo to be the same in this regard.
> >
> > Not exactly: The shared info page is special in any event. For
> > runstate info (and alike - there's also struct vcpu_time_info)
> > I'd question correctness of the current handling. If that's
> > wrong already, I'd prefer if the issue wasn't spread.
>
> There are also grants, which when used together with another guest on
> the same host could allow to bypass introspection AFAICT? (unless
> there's some policy applied that limit grant sharing to trusted
> domains)
>
> TBH I'm not sure how to handle hypoervisor accesses with
> introspection.  My knowledge of introspection is fairly limited, but
> it pauses the guest and sends a notification to an in guest agent. I'm
> not sure this is applicable to hypervisor writes, since it's not
> possible to pause hypervisor execution and wait for a response from a
> guest agent.
>

Introspection applications only care about memory accesses performed
by the guest. Hypervisor accesses to monitored pages are not included
when monitoring - it is actually a feature when using the emulator in
Xen to continue guest execution because the hypervisor ignores EPT
memory permissions that trip the guest for introspection. So having
the hypervisor access memory or a grant-shared page being accessed in
another domain are not a problem for introspection.

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/firmware: update OVMF Makefile

[Tamas K Lengyel]

On Wed, Jan 9, 2019 at 8:46 AM Anthony PERARD  wrote:
>
> On Wed, Jan 09, 2019 at 07:56:59AM -0700, Tamas K Lengyel wrote:
> > On Wed, Jan 9, 2019 at 7:56 AM Tamas K Lengyel
> >  wrote:
> > >
> > > On Wed, Nov 28, 2018 at 10:44 AM Wei Liu  wrote:
> > > >
> > > > OVMF has become dependent on OpenSSL, which it is included as a 
> > > > submodule.
> > > > Initialise submodules before building.
> > >
> > > If you are updating the ovmf makefile, could you by any chance also
> > > make the debug build of it more useful on Xen by making it print to
> > > the Xen console? Needs the -D DEBUG_ON_SERIAL_PORT flag added and the
> > > following one-liner to change it to correct port:
> > >
> > > sed -i 's/PcdDebugIoPort|0xe9/PcdDebugIoPort|0x402/g' OvmfPkg/OvmfPkg.dec
> > >
> >
> > Whops, actually the other way around for sed:
> >
> > sed -i 's/PcdDebugIoPort|0x402/PcdDebugIoPort|0xe9/g' OvmfPkg/OvmfPkg.dec
>
> You can actually have OVMF debug output without rebuilding it, add this
> to our VM config:
> device_model_args_hvm = [
>   # Debug OVMF
>   '-chardev', 
> 'file,id=debugcon,mux=on,path=/var/log/xen/qemu-dm-ovmf.log.debugcon,',
>   '-device', 'isa-debugcon,iobase=0x402,chardev=debugcon',
> ]
>
> That way OVMF boot isn't slow down by writing to an ioport if there
> isn't someone to care.
>
> The way you suggest will have OVMF write to an ioport that the Xen
> hypervisor will then write to it's console (or xl dmesg) and with the
> amount of debug that ovmf write, this is quite slow. And worse, that
> isn't going to work anymore with upstream OVMF as it now check if there
> is something to listen on the other side of the ioport, Xen isn't going
> answer the right things and OVMF will stay silence. (That paragraph is
> about the change of PcdDebugIoPort.)
>
> I don't know if DEBUG_ON_SERIAL_PORT is going to work, and how it works,
> but it certainly don't use PcdDebugIoPort. Using a serial port instead
> of an IO port is even going to be slower. So I don't know if it would be
> useful to have.

Ah, thanks for that info! We should probably put that on the wiki too ;)

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] Xen (both x86 and Arm) Community Call: Jan 9 - 16:00 - 17:00 UTC - Call for Agenda Items

[Rich Persaud]

On Jan 9, 2019, at 10:46, Lars Kurth  wrote:
> 
> Quick note: the meeting is in 15 minutes - the agenda is at 
> https://docs.google.com/document/d/1Ufv9XcQO0zIAVeFbFCAHAeEIB9Ap4Y4srAm4vI8I01I/edit
>  

On the topic of 4.12, I would like to propose moving the merge date by one 
week, since about 50% of Xen  maintainers who can review the Argo patch series 
for 4.12 are unavailable this week.

Rich___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Christopher Clark]

On Wed, Jan 9, 2019 at 7:56 AM Wei Liu  wrote:
>
> On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark wrote:
> > The register op is used by a domain to register a region of memory for
> > receiving messages from either a specified other domain, or, if specifying a
> > wildcard, any domain.
> >
> > This operation creates a mapping within Xen's private address space that
> > will remain resident for the lifetime of the ring. In subsequent commits,
> > the hypervisor will use this mapping to copy data from a sending domain into
> > this registered ring, making it accessible to the domain that registered the
> > ring to receive data.
> >
> > Wildcard any-sender rings are default disabled and registration will be
> > refused with EPERM unless they have been specifically enabled with the
> > argo-mac boot option introduced here. The reason why the default for
> > wildcard rings is 'deny' is that there is currently no means to protect the
> > ring from DoS by a noisy domain spamming the ring, affecting other domains
> > ability to send to it. This will be addressed with XSM policy controls in
> > subsequent work.
> >
> > Since denying access to any-sender rings is a significant functional
> > constraint, a new bootparam is provided to enable overriding this:
> >  "argo-mac" variable has allowed values: 'permissive' and 'enforcing'.
> > Even though this is a boolean variable, use these descriptive strings in
> > order to make it obvious to an administrator that this has potential
> > security impact.
> >
> > The p2m type of the memory supplied by the guest for the ring must be
> > p2m_ram_rw and the memory will be pinned as PGT_writable_page while the ring
> > is registered.
> >
> > xen_argo_page_descr_t type is introduced as a page descriptor, to convey
> > both the physical address of the start of the page and its granularity. The
> > smallest granularity page is assumed to be 4096 bytes and the lower twelve
> > bits of the type are used to indicate the size of page of memory supplied.
> > The implementation of the hypercall op currently only supports 4K pages.
> >
>
> What is the resolution for the Arm issues mentioned by Julien? I read
> the conversation in previous thread. A solution seemed to have been
> agreed upon, but the changelog doesn't say anything about it.

I made the interface changes that Julien had asked for. The register
op now takes arguments that can describe the granularitity of the
pages supplied, though only support for 4K pages is accepted in the
current implementation. I believe it meets Julien's requirements.

thanks,

Christopher

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH RFC v1] x86/emulate: Send vm_event form emulate

[Jan Beulich]

>>> On 09.01.19 at 16:47,  wrote:
> On Mon, Jan 7, 2019 at 2:11 PM Alexandru Stefan ISAILA 
>  wrote:
>> +
>> +req.reason = VM_EVENT_REASON_MEM_ACCESS;
>> +req.u.mem_access.gfn = gfn_x(gfn);
>> +req.u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA | 
>> MEM_ACCESS_GLA_VALID;
>> +req.u.mem_access.gla = gla;
>> +req.u.mem_access.offset = gpa & ((1 << PAGE_SHIFT) - 1);
>> +
>> +if ( monitor_traps(current, true, ) < 0 )
>> +return false;
>> +
>> +return true;
> 
> I think you can simplify this to:
> 
> return monitor_traps(current, true, ) < 0 ? false : true;

return monitor_traps(current, true, ) >= 0;

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 07/15] argo: implement the register op

[Wei Liu]

On Sun, Jan 06, 2019 at 11:42:40PM -0800, Christopher Clark wrote:
> The register op is used by a domain to register a region of memory for
> receiving messages from either a specified other domain, or, if specifying a
> wildcard, any domain.
> 
> This operation creates a mapping within Xen's private address space that
> will remain resident for the lifetime of the ring. In subsequent commits,
> the hypervisor will use this mapping to copy data from a sending domain into
> this registered ring, making it accessible to the domain that registered the
> ring to receive data.
> 
> Wildcard any-sender rings are default disabled and registration will be
> refused with EPERM unless they have been specifically enabled with the
> argo-mac boot option introduced here. The reason why the default for
> wildcard rings is 'deny' is that there is currently no means to protect the
> ring from DoS by a noisy domain spamming the ring, affecting other domains
> ability to send to it. This will be addressed with XSM policy controls in
> subsequent work.
> 
> Since denying access to any-sender rings is a significant functional
> constraint, a new bootparam is provided to enable overriding this:
>  "argo-mac" variable has allowed values: 'permissive' and 'enforcing'.
> Even though this is a boolean variable, use these descriptive strings in
> order to make it obvious to an administrator that this has potential
> security impact.
> 
> The p2m type of the memory supplied by the guest for the ring must be
> p2m_ram_rw and the memory will be pinned as PGT_writable_page while the ring
> is registered.
> 
> xen_argo_page_descr_t type is introduced as a page descriptor, to convey
> both the physical address of the start of the page and its granularity. The
> smallest granularity page is assumed to be 4096 bytes and the lower twelve
> bits of the type are used to indicate the size of page of memory supplied.
> The implementation of the hypercall op currently only supports 4K pages.
> 

What is the resolution for the Arm issues mentioned by Julien? I read
the conversation in previous thread. A solution seemed to have been
agreed upon, but the changelog doesn't say anything about it.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH RFC v1] x86/emulate: Send vm_event form emulate

[Roger Pau Monné]

On Mon, Jan 7, 2019 at 2:11 PM Alexandru Stefan ISAILA
 wrote:
>
> This patch aims to have mem access vm events sent from the emulator.
> This is useful in the case of page-walks that have to emulate
> instructions in access denied pages.
>
> We use hvmemul_map_linear_addr() ro intercept r/w access and
> hvmemul_insn_fetch() to intercept exec access.
>
> First we try to send a vm event and if the event is sent then emulation
> returns X86EMUL_ACCESS_EXCEPTION. If the event is not sent then the
> emulation goes on as expected.
>
> Signed-off-by: Alexandru Isaila 
> ---
>  xen/arch/x86/hvm/emulate.c | 298 +
>  xen/arch/x86/hvm/vm_event.c|   2 +-
>  xen/arch/x86/mm/mem_access.c   |   4 +-
>  xen/arch/x86/x86_emulate/x86_emulate.h |   1 +
>  xen/include/asm-x86/hvm/emulate.h  |   4 +-
>  5 files changed, 212 insertions(+), 97 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 2d02ef1521..f43aed379b 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -26,6 +27,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include "../mm/mm-locks.h"
>
>  static void hvmtrace_io_assist(const ioreq_t *p)
>  {
> @@ -530,6 +533,157 @@ static int hvmemul_do_mmio_addr(paddr_t mmio_gpa,
>  return hvmemul_do_io_addr(1, mmio_gpa, reps, size, dir, df, ram_gpa);
>  }
>
> +static bool hvmemul_send_vm_event(paddr_t gpa, unsigned long gla, gfn_t gfn,
> +  uint32_t pfec, struct hvm_emulate_ctxt 
> *ctxt)
> +{
> +p2m_access_t access = p2m_access_n;
> +struct p2m_domain *p2m = NULL;

No need to initialize to NULL or to p2m_access_n.

> +vm_event_request_t req = {};
> +p2m_type_t p2mt;
> +mfn_t mfn;
> +
> +if ( !ctxt->send_event || !pfec )
> +return false;
> +
> +p2m = p2m_get_hostp2m(current->domain);
> +if ( altp2m_active(current->domain) )
> +p2m = p2m_get_altp2m(current);
> +if ( !p2m )
> +p2m = p2m_get_hostp2m(current->domain);
> +
> +gfn_lock(p2m, gfn, 0);
> +mfn = p2m->get_entry(p2m, gfn, , , 0, NULL, NULL);
> +gfn_unlock(p2m, gfn, 0);

Don't you need to keep the gfn locked for at lest the duration of the
function? Or else what you put in the req struct below might not be
accurate by the time you write it. Maybe this is not relevant because
this req ends up queued in an async ring, so by the time the other end
reads the request the information might indeed have changed already.

Also, I'm wondering whether there's no helper to fetch the gfn entry
information from the p2m when using altp2m. The above construct (or
variations of it) must be widely used in altp2m code?

> +
> +if ( mfn_eq(mfn, INVALID_MFN) )
> +return false;
> +
> +switch ( access ) {
> +case p2m_access_x:
> +case p2m_access_rx:
> +if ( pfec & PFEC_write_access )
> +req.u.mem_access.flags = MEM_ACCESS_R | MEM_ACCESS_W;
> +break;

Newline.

> +case p2m_access_w:
> +case p2m_access_rw:
> +if ( pfec & PFEC_insn_fetch )
> +req.u.mem_access.flags = MEM_ACCESS_X;
> +break;

Newline.

> +case p2m_access_r:
> +case p2m_access_n:
> +if ( pfec & PFEC_write_access )
> +req.u.mem_access.flags |= MEM_ACCESS_R | MEM_ACCESS_W;
> +if ( pfec & PFEC_insn_fetch )
> +req.u.mem_access.flags |= MEM_ACCESS_X;
> +break;

Newline.

> +default:
> +return false;
> +}

I'm not sure the switch is needed, you can't have a PFEC_write_access
for example if the p2m type is p2m_access_w or p2m_access_rw, hence it
seems like it could be simplified to only take the pfec into account?

> +
> +if ( !req.u.mem_access.flags )
> +return false; //no violation

Wrong comment style.

> +
> +req.reason = VM_EVENT_REASON_MEM_ACCESS;
> +req.u.mem_access.gfn = gfn_x(gfn);
> +req.u.mem_access.flags |= MEM_ACCESS_FAULT_WITH_GLA | 
> MEM_ACCESS_GLA_VALID;
> +req.u.mem_access.gla = gla;
> +req.u.mem_access.offset = gpa & ((1 << PAGE_SHIFT) - 1);
> +
> +if ( monitor_traps(current, true, ) < 0 )
> +return false;
> +
> +return true;

I think you can simplify this to:

return monitor_traps(current, true, ) < 0 ? false : true;

> +}
> +
> +/*
> + * Convert addr from linear to physical form, valid over the range
> + * [addr, addr + *reps * bytes_per_rep]. *reps is adjusted according to
> + * the valid computed range. It is always >0 when X86EMUL_OKAY is returned.
> + * @pfec indicates the access checks to be performed during page-table walks.
> +*/
> +static int hvmemul_linear_to_phys(
> +unsigned long addr,
> +paddr_t *paddr,
> +unsigned int bytes_per_rep,
> +unsigned long *reps,
> +uint32_t pfec,
> +struct hvm_emulate_ctxt *hvmemul_ctxt)
> +{

Re: [Xen-devel] Xen (both x86 and Arm) Community Call: Jan 9 - 16:00 - 17:00 UTC - Call for Agenda Items

[Lars Kurth]

Quick note: the meeting is in 15 minutes - the agenda is at 
https://docs.google.com/document/d/1Ufv9XcQO0zIAVeFbFCAHAeEIB9Ap4Y4srAm4vI8I01I/edit
 

 

> On 8 Jan 2019, at 14:34, Lars Kurth  wrote:
> 
> Hi all,
> 
> I noticed that I hadn't updated all the times in the meeting invite block.
> 
>> 
>> == Dial-in Information ==
>> 
>>   ## Future Community Call schedule
>>   Jan 9, Feb 13, Mar 12
>> 
>>   ## Meeting time
>>   15:00 - 16:00 UTC
> 
> This is wrong and should read 16:00 - 17:00
> 
>>8:00 -  9:00 EDT (San Francisco)
>>   11:00 - 12:00 EDT (New York)
>>   16:00 - 17:00 BST (London)
>>   17:00 - 18:00 CEST (Berlin)
>>   00:00 - 01:00 CST (Beijing)
>>   Further International meeting times: 
>>   
>> https://www.timeanddate.com/worldclock/meetingdetails.html?year=2019=1=7=16=0=0=224=24=179=136=37=33
>>  
>> 
> 
> Regards
> Lars
> 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Wed, Jan 09, 2019 at 07:56:59AM -0700, Tamas K Lengyel wrote:
> On Wed, Jan 9, 2019 at 7:56 AM Tamas K Lengyel
>  wrote:
> >
> > On Wed, Nov 28, 2018 at 10:44 AM Wei Liu  wrote:
> > >
> > > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > > Initialise submodules before building.
> >
> > If you are updating the ovmf makefile, could you by any chance also
> > make the debug build of it more useful on Xen by making it print to
> > the Xen console? Needs the -D DEBUG_ON_SERIAL_PORT flag added and the
> > following one-liner to change it to correct port:
> >
> > sed -i 's/PcdDebugIoPort|0xe9/PcdDebugIoPort|0x402/g' OvmfPkg/OvmfPkg.dec
> >
> 
> Whops, actually the other way around for sed:
> 
> sed -i 's/PcdDebugIoPort|0x402/PcdDebugIoPort|0xe9/g' OvmfPkg/OvmfPkg.dec

You can actually have OVMF debug output without rebuilding it, add this
to our VM config:
device_model_args_hvm = [
  # Debug OVMF
  '-chardev', 
'file,id=debugcon,mux=on,path=/var/log/xen/qemu-dm-ovmf.log.debugcon,',
  '-device', 'isa-debugcon,iobase=0x402,chardev=debugcon',
]

That way OVMF boot isn't slow down by writing to an ioport if there
isn't someone to care.

The way you suggest will have OVMF write to an ioport that the Xen
hypervisor will then write to it's console (or xl dmesg) and with the
amount of debug that ovmf write, this is quite slow. And worse, that
isn't going to work anymore with upstream OVMF as it now check if there
is something to listen on the other side of the ioport, Xen isn't going
answer the right things and OVMF will stay silence. (That paragraph is
about the change of PcdDebugIoPort.)

I don't know if DEBUG_ON_SERIAL_PORT is going to work, and how it works,
but it certainly don't use PcdDebugIoPort. Using a serial port instead
of an IO port is even going to be slower. So I don't know if it would be
useful to have.

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [linux-3.18 test] 131800: regressions - trouble: broken/fail/pass

[osstest service owner]

flight 131800 linux-3.18 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131800/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-multivcpu broken
 test-amd64-i386-rumprun-i386 broken  in 131780
 test-amd64-i386-qemut-rhel6hvm-intel  broken in 131780
 test-amd64-amd64-xl-qemuu-ovmf-amd64  broken in 131780
 test-amd64-i386-xl-qemut-win7-amd64   broken in 131780
 test-amd64-i386-qemuu-rhel6hvm-intel  broken in 131780
 test-amd64-i386-xl-shadowbroken  in 131780
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 128858
 test-amd64-i386-rumprun-i386  7 xen-boot fail REGR. vs. 128858
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 128858
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. 
vs. 128858
 test-amd64-amd64-qemuu-nested-intel  7 xen-boot  fail REGR. vs. 128858
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. 
vs. 128858
 test-amd64-amd64-xl-multivcpu  7 xen-bootfail REGR. vs. 128858
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 128858
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 128858
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 128858
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 128858
 test-amd64-amd64-rumprun-amd64  7 xen-boot   fail REGR. vs. 128858
 test-amd64-amd64-i386-pvgrub  7 xen-boot fail REGR. vs. 128858
 test-amd64-amd64-xl-xsm   7 xen-boot fail REGR. vs. 128858
 test-amd64-amd64-pair10 xen-boot/src_hostfail REGR. vs. 128858
 test-amd64-amd64-pair11 xen-boot/dst_hostfail REGR. vs. 128858
 test-amd64-amd64-libvirt-pair 10 xen-boot/src_host   fail REGR. vs. 128858
 test-amd64-amd64-libvirt-pair 11 xen-boot/dst_host   fail REGR. vs. 128858
 test-amd64-amd64-xl-qemuu-ovmf-amd64  7 xen-boot fail REGR. vs. 128858
 test-amd64-amd64-xl-pvshim7 xen-boot fail REGR. vs. 128858
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 128858
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 128858
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
128858

Tests which are failing intermittently (not blocking):
 test-amd64-i386-qemut-rhel6hvm-intel 4 host-install(4) broken in 131780 pass 
in 131800
 test-amd64-i386-qemuu-rhel6hvm-intel 4 host-install(4) broken in 131780 pass 
in 131800
 test-amd64-i386-rumprun-i386 4 host-install(4) broken in 131780 pass in 131800
 test-amd64-i386-xl-shadow4 host-install(4) broken in 131780 pass in 131800
 test-amd64-amd64-xl-qemuu-ovmf-amd64 4 host-install(4) broken in 131780 pass 
in 131800
 test-amd64-i386-xl-qemut-win7-amd64 4 host-install(4) broken in 131780 pass in 
131800
 test-armhf-armhf-xl-multivcpu  4 host-install(4) broken pass in 131780
 test-armhf-armhf-xl-multivcpu 16 guest-start/debian.repeat fail in 131535 pass 
in 131780
 test-armhf-armhf-xl-credit1 16 guest-start/debian.repeat fail in 131563 pass 
in 131800
 test-amd64-i386-libvirt-xsm 18 guest-start/debian.repeat fail in 131580 pass 
in 131800
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail in 131666 pass in 131800
 test-amd64-i386-xl-xsm7 xen-boot fail in 131666 pass in 131800
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail in 131666 
pass in 131800
 test-armhf-armhf-libvirt  6 xen-install  fail in 131666 pass in 131800
 test-amd64-i386-libvirt  21 leak-check/check fail in 131705 pass in 131800
 test-armhf-armhf-xl-multivcpu  7 xen-bootfail in 131749 pass in 131780
 test-armhf-armhf-libvirt 17 guest-start.2fail in 131769 pass in 131780
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail in 
131769 pass in 131800
 test-amd64-amd64-xl-credit2   7 xen-boot   fail pass in 131535
 test-amd64-amd64-xl   7 xen-boot   fail pass in 131535
 test-amd64-amd64-amd64-pvgrub  7 xen-boot  fail pass in 131563
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_host  fail pass in 131580
 test-amd64-i386-xl-shadow 7 xen-boot   fail pass in 131580
 test-amd64-i386-freebsd10-i386  7 xen-boot fail pass in 131580
 test-amd64-amd64-xl-rtds  7 xen-boot   fail pass in 131666
 test-amd64-amd64-libvirt-xsm  7 xen-boot   fail pass in 131705
 test-amd64-i386-xl-raw7 xen-boot   fail pass in 131729
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  7 xen-boot  fail pass in 131749
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail pass in 

Re: [Xen-devel] [PATCH] tmem: default to off

[Jan Beulich]

>>> On 09.01.19 at 16:02,  wrote:
> From: Xen-devel  on behalf of Jan 
> Beulich 
> :
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -21,7 +21,7 @@ obj-$(CONFIG_KEXEC) += kimage.o
>  obj-y += lib.o
>  obj-$(CONFIG_NEEDS_LIST_SORT) += list_sort.o
>  obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
> -obj-y += lzo.o
> +obj-$(CONFIG_TMEM) += lzo.o
> 
> Here you completely disable the build of lzo if tmem is not enabled.
> 
>  obj-$(CONFIG_MEM_ACCESS) += mem_access.o
>  obj-y += memory.o
>  obj-y += monitor.o
> @@ -66,8 +66,9 @@ obj-bin-y += warning.init.o
>  obj-$(CONFIG_XENOPROF) += xenoprof.o
>  obj-y += xmalloc_tlsf.o
> 
> -obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo 
> unlz4 earlycpio,$(n).init.o)
> -
> +lzo-y := lzo
> +lzo-$(CONFIG_TMEM) :=
> +obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma $(lzo-y) 
> unlzo unlz4 earlycpio,$(n).init.o)
> 
> Here however you always build unlzo.c, which AFAICT makes use of the 
> lzo1x_decompress_safe function that's defined in lzo.c.

Note the (new) definition and use of lzo-y, so together with unlzo.init.o
lzo.init.o will also be built. (I can assure you that I did test with TMEM
enabled and disabled.)

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [qemu-mainline test] 131842: tolerable FAIL - PUSHED

[osstest service owner]

flight 131842 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131842/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 131801
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 131801
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 131801
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 131801
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 131801
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   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-amd64-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-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-libvirt 13 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-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-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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-raw 12 migrate-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-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
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass

version targeted for testing:
 qemuu147923b1a901a0370f83a0f4c58ec1baffef22f0
baseline version:
 qemuuc102d9471f8f02d9fbea72ec4505d7089173f470

Last test of basis   131801  2019-01-08 09:39:41 Z1 days
Testing same since   131842  2019-01-09 00:37:22 Z0 days1 attempts


People who touched revisions under test:
  Daniel P. Berrangé 
  Gerd Hoffmann 
  Hongbo Zhang 
  Jonathan Davies 
  Li Qiang 
  Markus Armbruster 
  Paolo Bonzini 
  Peter Maydell 
  Philippe Mathieu-Daudé 
  Richard Henderson 
  Roman Bolshakov 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 test-amd64-amd64-libvirt-xsm pass
 

Re: [Xen-devel] [PATCH] tmem: default to off

[Roger Pau Monne]

Sorry for the wrong formatting.

From: Xen-devel  on behalf of Jan 
Beulich :
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -21,7 +21,7 @@ obj-$(CONFIG_KEXEC) += kimage.o
 obj-y += lib.o
 obj-$(CONFIG_NEEDS_LIST_SORT) += list_sort.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
-obj-y += lzo.o
+obj-$(CONFIG_TMEM) += lzo.o

Here you completely disable the build of lzo if tmem is not enabled.

 obj-$(CONFIG_MEM_ACCESS) += mem_access.o
 obj-y += memory.o
 obj-y += monitor.o
@@ -66,8 +66,9 @@ obj-bin-y += warning.init.o
 obj-$(CONFIG_XENOPROF) += xenoprof.o
 obj-y += xmalloc_tlsf.o

-obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo 
unlz4 earlycpio,$(n).init.o)
-
+lzo-y := lzo
+lzo-$(CONFIG_TMEM) :=
+obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma $(lzo-y) 
unlzo unlz4 earlycpio,$(n).init.o)

Here however you always build unlzo.c, which AFAICT makes use of the 
lzo1x_decompress_safe function that's defined in lzo.c.

Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/firmware: update OVMF Makefile

[Tamas K Lengyel]

On Wed, Jan 9, 2019 at 7:56 AM Tamas K Lengyel
 wrote:
>
> On Wed, Nov 28, 2018 at 10:44 AM Wei Liu  wrote:
> >
> > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > Initialise submodules before building.
>
> If you are updating the ovmf makefile, could you by any chance also
> make the debug build of it more useful on Xen by making it print to
> the Xen console? Needs the -D DEBUG_ON_SERIAL_PORT flag added and the
> following one-liner to change it to correct port:
>
> sed -i 's/PcdDebugIoPort|0xe9/PcdDebugIoPort|0x402/g' OvmfPkg/OvmfPkg.dec
>

Whops, actually the other way around for sed:

sed -i 's/PcdDebugIoPort|0x402/PcdDebugIoPort|0xe9/g' OvmfPkg/OvmfPkg.dec

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/firmware: update OVMF Makefile

[Tamas K Lengyel]

On Wed, Nov 28, 2018 at 10:44 AM Wei Liu  wrote:
>
> OVMF has become dependent on OpenSSL, which it is included as a submodule.
> Initialise submodules before building.

If you are updating the ovmf makefile, could you by any chance also
make the debug build of it more useful on Xen by making it print to
the Xen console? Needs the -D DEBUG_ON_SERIAL_PORT flag added and the
following one-liner to change it to correct port:

sed -i 's/PcdDebugIoPort|0xe9/PcdDebugIoPort|0x402/g' OvmfPkg/OvmfPkg.dec

Tamas

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.12 V2] x86/p2m: fix p2m_finish_type_change()

[Jan Beulich]

>>> On 09.01.19 at 13:24,  wrote:
> finish_type_change() returns a negative int on error, but the
> current code checks if ( !rc ). We also need to treat
> finish_type_change()'s return codes cumulatively in the
> success case (don't overwrite a 1 returned while processing
> the hostp2m if processing an altp2m returns 0).
> 
> The breakage was introduced by commit 0fb4b58c8b
> ("x86/altp2m: fix display frozen when switching to a new view
> early").
> 
> Properly indent the out: label while at it.
> 
> Signed-off-by: Razvan Cojocaru 
> 
> ---
> Changes since V1:
>  - Updated description.
>  - Now treating finish_type_change()'s return value cumulatively
>for the success case.
> ---
>  xen/arch/x86/mm/p2m.c | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index 5451f16..91f412f 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
>  
>  rc = finish_type_change(hostp2m, first_gfn, max_nr);
>  
> -if ( !rc )
> +if ( rc < 0 )
>  goto out;
>  
>  #ifdef CONFIG_HVM
> @@ -1188,18 +1188,24 @@ int p2m_finish_type_change(struct domain *d,
>  if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
>  {
>  struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
> +int rc1;
>  
>  p2m_lock(altp2m);
> -rc = finish_type_change(altp2m, first_gfn, max_nr);
> +rc1 = finish_type_change(altp2m, first_gfn, max_nr);
>  p2m_unlock(altp2m);
>  
> -if ( !rc )
> +if ( rc1 < 0 )
> +{
> +rc = rc1;
>  goto out;
> +}
> +
> +rc = max(rc, rc1);

"rc |= rc1" would likely be cheaper in terms of generated code. But
functionally this is fine, and the change would be easy enough to
make while committing, so
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 v3 04/15] argo: init, destroy and soft-reset, with enable command line opt

[Jan Beulich]

>>> On 09.01.19 at 15:26,  wrote:
> On Wed, Jan 9, 2019 at 4:35 AM Jan Beulich  wrote:
>> >>> On 08.01.19 at 23:54,  wrote:
>> > On Mon, Jan 7, 2019 at 2:43 AM Christopher Clark 
>> >  wrote:
>> >> + */
>> >> +struct argo_ring_info *ring_info;
>> >> +/* domain to be notified when space is available */
>> >> +domid_t domain_id;
>> >> +uint16_t pad;
>> >
>> > Can we order domain_id after len and drop the pad?
>>
>> That would still call for a pad field - we prefer to have explicit padding,
>> and also to check it's zero, the latter to allow for assigning meaning to
>> the field down the road.
> 
> This struct is internal to Xen and argo, so do we still need explicit 
> padding?

Oh, internal structures don't need any explicit padding. Where the
domain_id field gets placed still doesn't matter then, though.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] sched/credit2: remove stale comment

[Juergen Gross]

On 09/01/2019 15:27, Jan Beulich wrote:
 On 09.01.19 at 14:59,  wrote:
>> On Wed, 2019-01-09 at 13:34 +0100, Juergen Gross wrote:
>>> With being the default scheduler now the comment in sched_credit2
>>> stating it being experimental should be removed.
>>>
>>> While at it remove the "TODO" comments already addressed.
>>>
>>> Signed-off-by: Juergen Gross 
>>>
>> Acked-by: Dario Faggioli 
> 
> And I guess this is fine to put in for 4.12?

Yes:

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] sched/credit2: remove stale comment

[Jan Beulich]

>>> On 09.01.19 at 14:59,  wrote:
> On Wed, 2019-01-09 at 13:34 +0100, Juergen Gross wrote:
>> With being the default scheduler now the comment in sched_credit2
>> stating it being experimental should be removed.
>> 
>> While at it remove the "TODO" comments already addressed.
>> 
>> Signed-off-by: Juergen Gross 
>>
> Acked-by: Dario Faggioli 

And I guess this is fine to put in for 4.12?

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 04/15] argo: init, destroy and soft-reset, with enable command line opt

[Jason Andryuk]

On Wed, Jan 9, 2019 at 4:35 AM Jan Beulich  wrote:
>
> >>> On 08.01.19 at 23:54,  wrote:
>
> First of all - please trim your replies.

Sorry.  Will do.

> > On Mon, Jan 7, 2019 at 2:43 AM Christopher Clark 
> >  wrote:
> >> --- a/docs/misc/xen-command-line.pandoc
> >> +++ b/docs/misc/xen-command-line.pandoc
> >> @@ -182,6 +182,17 @@ Permit Xen to use "Always Running APIC Timer" support 
> >> on compatible hardware
> >>  in combination with cpuidle.  This option is only expected to be useful 
> >> for
> >>  developers wishing Xen to fall back to older timing methods on newer 
> >> hardware.
> >>
> >> +### argo
> >> +> `= `
> >> +
> >> +> Default: `false`
> >> +
> >> +Enable the Argo hypervisor-mediated interdomain communication mechanism.
> >> +
> >> +This allows domains access to the Argo hypercall, which supports 
> >> registration
> >> +of memory rings with the hypervisor to receive messages, sending messages 
> >> to
> >> +other domains by hypercall and querying the ring status of other domains.
> >> +
> >
> > Do we want to say it's only available when Xen is compiled with CONFIG_ARGO?
>
> We don't do so elsewhere, so I'm with Christopher.

Okay.

> >> + */
> >> +struct argo_ring_info *ring_info;
> >> +/* domain to be notified when space is available */
> >> +domid_t domain_id;
> >> +uint16_t pad;
> >
> > Can we order domain_id after len and drop the pad?
>
> That would still call for a pad field - we prefer to have explicit padding,
> and also to check it's zero, the latter to allow for assigning meaning to
> the field down the road.

This struct is internal to Xen and argo, so do we still need explicit padding?

There are other public argo structs with padding fields.  I haven't
gotten through all the patches, but I think at least some of those are
missing zero checks.

Regards,
Jason

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 04/15] argo: init, destroy and soft-reset, with enable command line opt

[Jason Andryuk]

On Wed, Jan 9, 2019 at 1:48 AM Christopher Clark
 wrote:
>
> On Tue, Jan 8, 2019 at 2:54 PM Jason Andryuk  wrote:
> >
> > On Mon, Jan 7, 2019 at 2:43 AM Christopher Clark
> >  wrote:

> > > +
> > > +/* A space-available notification that is awaiting sufficient space */
> > > +struct pending_ent
> > > +{
> > > +/* List node within argo_ring_info's pending list */
> > > +struct hlist_node node;
> > > +/*
> > > + * List node within argo_domain's wildcard_pend_list. Only used if 
> > > the
> > > + * ring is one with a wildcard partner (ie. that any domain may send 
> > > to)
> > > + * to enable cancelling signals on wildcard rings on domain destroy.
> > > + */
> > > +struct hlist_node wildcard_node;
> > > +/*
> > > + * Pointer to the ring_info that this ent pertains to. Used to 
> > > ensure that
> > > + * ring_info->npending is decremented when ents for wildcard rings 
> > > are
> > > + * cancelled for domain destroy.
> > > + * Caution: Must hold the correct locks before accessing ring_info 
> > > via this.
> >
> > It would be clearer if this stated the correct locks.
>
> ok - it would mean duplicating the statement about which locks are
> needed though, since it is explained elsewhere in the file, which means
> it will need updating in two places if the locking requirements change.
> That was why I worded it that way, as an indicator to go and find where
> it is already described, to avoid that.

"Caution" made me think *ring_info points from domain A's pending_ent
to domain B's ring_info.  Reading patch 10 (notify op) I see that it
really just points back to domain A's ring_info.  So the "Caution" is
just that you still have to lock ring_info (L3) even though you can
get to the pointer via L2.  Is that correct?

I agree a single location for the locking documentation is better than
splitting or duplicating.  As long as no cross-domain locking is
required, this is fine.

> > > + */
> > > +struct argo_ring_info *ring_info;
> > > +/* domain to be notified when space is available */
> > > +domid_t domain_id;
> > > +uint16_t pad;
> >
> > Can we order domain_id after len and drop the pad?
>
> I'm not sure that would be right to do that. I think that the pad
> ensures that len is word aligned to 32-bit boundary.  I was asked to
> insert a pad field for such a struct like this in an earlier review here:
>
> https://lists.xenproject.org/archives/html/xen-devel/2018-12/msg00239.html

I'll respond to this in the other email from Jan.

> > > +
> > > +static void
> > > +wildcard_rings_pending_remove(struct domain *d)
> > > +{
> > > +struct hlist_node *node, *next;
> > > +struct pending_ent *ent;
> > > +
> > > +ASSERT(rw_is_write_locked(_lock));
> > > +
> > > +hlist_for_each_entry_safe(ent, node, next, 
> > > >argo->wildcard_pend_list,
> > > +  node)
> > > +{
> > > +hlist_del(>node);
> > > +ent->ring_info->npending--;
> > > +hlist_del(>wildcard_node);
> > > +xfree(ent);
> > > +}
> > > +}
> > > +
> >
> > Maybe move ring_unmap() here so it's closer to where it is used?
>
> I'm fine with moving it if it needs it, but it's located where it is in
> order to put it right next to the corresponding ring_map_page function -
> the two are paired really, with one doing map_domain_page_global and the
> other undoing it with unmap_domain_page_global. That's how it ends up
> when the full series is applied.

Okay.  My comment came only from reading only this single patch.

> > > +void
> > > +argo_soft_reset(struct domain *d)
> > > +{
> > > +write_lock(_lock);
> > > +
> > > +argo_dprintk("soft reset d=%d d->argo=%p\n", d->domain_id, d->argo);
> > > +
> > > +if ( d->argo )
> > > +{
> > > +domain_rings_remove_all(d);
> > > +partner_rings_remove(d);
> > > +wildcard_rings_pending_remove(d);
> > > +
> > > +if ( !opt_argo_enabled )
> >
> > Shouldn't this function just exit early if argo is disabled?
>
> There has been support added to Xen with a hypercall to make a subset of
> boot parameters modifiable at runtime. Argo-enabled isn't currently one
> of them, but that may be changed later so I did not want to bake into
> this function the assumption that the enabled/disabled configuration
> could not change after being initially evaluated at the time the domain
> was launched. That's possibly a conservative choice though.
>
> http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=82cf78468e96de1e4d1400bbf5508f8b111650c3

Okay.  I was not aware of this functionality.

Regards,
Jason

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] sched/credit2: remove stale comment

[Dario Faggioli]

On Wed, 2019-01-09 at 13:34 +0100, Juergen Gross wrote:
> With being the default scheduler now the comment in sched_credit2
> stating it being experimental should be removed.
> 
> While at it remove the "TODO" comments already addressed.
> 
> Signed-off-by: Juergen Gross 
>
Acked-by: Dario Faggioli 

Thanks,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/


signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [xen-unstable-smoke test] 131879: tolerable all pass - PUSHED

[osstest service owner]

flight 131879 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131879/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 13 migrate-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  f35a59452d5f5da20bef8acdc25f837ac782071c
baseline version:
 xen  1292f9a05943d32ef96eabb9f0c30cf681665c46

Last test of basis   131829  2019-01-08 19:00:36 Z0 days
Testing same since   131879  2019-01-09 12:00:34 Z0 days1 attempts


People who touched revisions under test:
  Anthony PERARD 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 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
   1292f9a059..f35a59452d  f35a59452d5f5da20bef8acdc25f837ac782071c -> smoke

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [ovmf test] 131861: regressions - FAIL

[osstest service owner]

flight 131861 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131861/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i3866 xen-buildfail REGR. vs. 129475
 build-i386-xsm6 xen-buildfail REGR. vs. 129475
 build-amd64-xsm   6 xen-buildfail REGR. vs. 129475
 build-amd64   6 xen-buildfail REGR. vs. 129475

Tests which did not succeed, but are not blocking:
 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
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf a53a888de8f5fa8dbf75a381e28f25a5497572f1
baseline version:
 ovmf 5ae3184d8c59f7bbb84bad482df6b8020ba58188

Last test of basis   129475  2018-11-05 21:13:11 Z   64 days
Failing since129526  2018-11-06 20:49:26 Z   63 days  256 attempts
Testing same since   131796  2019-01-08 08:06:26 Z1 days5 attempts


People who touched revisions under test:
  Achin Gupta 
  Alex James 
  Ard Biesheuvel 
  Ashish Singhal 
  Bob Feng 
  bob.c.f...@intel.com 
  BobCF 
  Chasel Chiu 
  Chasel, Chiu 
  Chen A Chen 
  Chu, Maggie 
  Dandan Bi 
  David Wei 
  Derek Lin 
  Eric Dong 
  Feng, Bob C 
  Fu Siyuan 
  Gary Lin 
  Hao Wu 
  Jaben Carsey 
  Jagadeesh Ujja 
  Jeff Brasen 
  Jian J Wang 
  Jiaxin Wu 
  Jiewen Yao 
  Laszlo Ersek 
  Leif Lindholm 
  Liming Gao 
  Liu Yu 
  Maggie Chu 
  Marc Zyngier 
  Marcin Wojtas 
  Mike Maslenkin 
  Ming Huang 
  Pedroa Liu 
  Ruiyu Ni 
  Sami Mujawar 
  shenglei 
  Shenglei Zhang 
  Siyuan Fu 
  Star Zeng 
  Sughosh Ganu 
  Sumit Garg 
  Sun, Zailiang 
  Thomas Abraham 
  Thomas Rydman 
  Ting Ye 
  Tomasz Michalec 
  Vijayenthiran Subramaniam 
  Vladimir Olovyannikov 
  Wang BinX A 
  Wu Jiaxin 
  Ye Ting 
  Yonghong Zhu 
  yuchenlin 
  Zailiang Sun 
  Zhang, Chao B 
  Zhao, ZhiqiangX 
  Zhiju.Fan 
  zhijufan 
  ZhiqiangX Zhao 
  zwei4 

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.

(No revision log; it would be 5677 lines long.)

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH] sched/credit2: remove stale comment

[Juergen Gross]

With being the default scheduler now the comment in sched_credit2
stating it being experimental should be removed.

While at it remove the "TODO" comments already addressed.

Signed-off-by: Juergen Gross 
---
 xen/common/sched_credit2.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 623a325ceb..543dc3664d 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -59,19 +59,13 @@
 #define TRC_CSCHED2_RUNQ_CAND_CHECK  TRC_SCHED_CLASS_EVT(CSCHED2, 23)
 
 /*
- * WARNING: This is still in an experimental phase.  Status and work can be 
found at the
- * credit2 wiki page:
- *  http://wiki.xen.org/wiki/Credit2_Scheduler_Development
- *
  * TODO:
  * + Hyperthreading
- *  - Look for non-busy core if possible
  *  - "Discount" time run on a thread with busy siblings
  * + Algorithm:
  *  - "Mixed work" problem: if a VM is playing audio (5%) but also burning cpu 
(e.g.,
  *a flash animation in the background) can we schedule it with low enough 
latency
  *so that audio doesn't skip?
- *  - Cap and reservation: How to implement with the current system?
  * + Optimizing
  *  - Profiling, making new algorithms, making math more efficient (no long 
division)
  */
-- 
2.16.4


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v9 16/18] xen: automatically create XenBlockDevice-s

[Paul Durrant]

> -Original Message-
> From: Anthony PERARD [mailto:anthony.per...@citrix.com]
> Sent: 09 January 2019 12:09
> To: Paul Durrant 
> Cc: qemu-de...@nongnu.org; qemu-bl...@nongnu.org; xen-
> de...@lists.xenproject.org; Kevin Wolf ; Max Reitz
> ; Stefano Stabellini 
> Subject: Re: [PATCH v9 16/18] xen: automatically create XenBlockDevice-s
> 
> On Tue, Jan 08, 2019 at 02:49:01PM +, Paul Durrant wrote:
> > This patch adds create and destroy function for XenBlockDevice-s so that
> > they can be created automatically when the Xen toolstack instantiates a
> new
> > PV backend via xenstore. When the XenBlockDevice is created this way it
> is
> > also necessary to create a 'drive' which matches the configuration that
> the
> > Xen toolstack has written into xenstore. This is done by formulating the
> > parameters necessary for each 'blockdev' layer of the drive and then
> using
> > qmp_blockdev_add() to create the layers. Also, for compatibility with
> the
> > legacy 'xen_disk' implementation, an iothread is automatically created
> for
> > the new XenBlockDevice. This, like the driver layers, will be destroyed
> > after the XenBlockDevice is unrealized.
> >
> > The legacy backend scan for 'qdisk' is removed by this patch, which
> makes
> > the 'xen_disk' code is redundant. The code will be removed by a
> subsequent
> > patch.
> >
> > Signed-off-by: Paul Durrant 
> > Signed-off-by: Anthony Perard 
> > ---
> 
> > diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> > index a7c37c185a..c6ec1d9543 100644
> > --- a/hw/block/xen-block.c
> > +++ b/hw/block/xen-block.c
> > @@ -7,12 +7,20 @@
> ...
> 
> > +static XenBlockDrive *xen_block_drive_create(const char *id,
> > + const char *device_type,
> > + QDict *opts, Error **errp)
> > +{
> ...
> > +Error *local_err = NULL;
> ...
> > +if (!filename) {
> > +error_setg(errp, "no filename");
> > +goto done;
> > +}
> ...
> > +drive->node_name = xen_block_blockdev_add(drive->id, driver_layer,
> > +  _err);
> > +
> > +done:
> > +g_free(driver);
> > +g_free(filename);
> > +
> > +if (local_err) {
> 
> When xen_block_blockdev_add failed, it sets local_err, but nothing after
> sets errp. I'm going to add this while preparing the pull request:
> 
> error_propagate(errp, local_err);
> 
> Is this fine with you?

Yes, that's fine... the error should have indeed been propagated.

> 
> With that fix, I think the series is ready, so:
> Reviewed-by: Anthony PERARD 
> 

Thanks.

> > +xen_block_drive_destroy(drive, NULL);
> > +return NULL;
> > +}
> > +
> > +return drive;
> > +}
> 
> There is still the warning about using the 'file' driver when
> 'host_device' should be use, but I think we can try to fix that later.

TBH I'm hoping this code can go away before we have to worry about it. We need 
to teach libxl how to issue the necessary QMP commands directly; it should know 
the difference between a file and a device.

  Paul

> 
> Thanks,
> 
> --
> Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH for-4.12 V2] x86/p2m: fix p2m_finish_type_change()

[Razvan Cojocaru]

finish_type_change() returns a negative int on error, but the
current code checks if ( !rc ). We also need to treat
finish_type_change()'s return codes cumulatively in the
success case (don't overwrite a 1 returned while processing
the hostp2m if processing an altp2m returns 0).

The breakage was introduced by commit 0fb4b58c8b
("x86/altp2m: fix display frozen when switching to a new view
early").

Properly indent the out: label while at it.

Signed-off-by: Razvan Cojocaru 

---
Changes since V1:
 - Updated description.
 - Now treating finish_type_change()'s return value cumulatively
   for the success case.
---
 xen/arch/x86/mm/p2m.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 5451f16..91f412f 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
 
 rc = finish_type_change(hostp2m, first_gfn, max_nr);
 
-if ( !rc )
+if ( rc < 0 )
 goto out;
 
 #ifdef CONFIG_HVM
@@ -1188,18 +1188,24 @@ int p2m_finish_type_change(struct domain *d,
 if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
 {
 struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
+int rc1;
 
 p2m_lock(altp2m);
-rc = finish_type_change(altp2m, first_gfn, max_nr);
+rc1 = finish_type_change(altp2m, first_gfn, max_nr);
 p2m_unlock(altp2m);
 
-if ( !rc )
+if ( rc1 < 0 )
+{
+rc = rc1;
 goto out;
+}
+
+rc = max(rc, rc1);
 }
 }
 #endif
 
-out:
+ out:
 p2m_unlock(hostp2m);
 
 return rc;
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC PATCH v2 11/17] xenconsoled: add support for consoles using 'state' xenstore entry

[Marek Marczykowski-Górecki]

On Thu, Nov 01, 2018 at 05:21:39PM +, Ian Jackson wrote:
> Marek Marczykowski-Górecki writes ("[RFC PATCH v2 11/17] xenconsoled: add 
> support for consoles using 'state' xenstore entry"):
> > Add support for standard xenbus initialization protocol using 'state'
> > xenstore entry. It will be necessary for secondary consoles.
> > For consoles supporting it, read 'state' entry on the frontend and
> > proceed accordingly - either init console or close it. When closing,
> > make sure all the in-transit data is flushed (both from shared ring and
> > from local buffer), if possible. This is especially important for
> > MiniOS-based qemu stubdomain, which closes console just after writing
> > device model state to it.
> > For consoles without 'state' field behavior is unchanged - on any watch
> > event try to connect console, as long as domain is alive.
> 
> I'm not opposed to the introduction of this state field.  The code
> looks plausible.
> 
> But:
> 
> Firstly, you have put the protocol description in your commit
> message (and it seems rather informal).  Can you please provide
> a comprehensive protocol specification in-tree ?  You need to patch
>docs/misc/console.txt
> I think.
> 
> I say `comprehensive' because your text is not particularly clearly
> about who is supposed to `flush' which data exactly when.  Nor what
> `flushing' means (does it ever mean discarding?)
> 
> Secondly: what about backwards compatibility ?  I think we need to at
> least think about the possibility that there are some guest frontends
> out there which may look for a `state' node and do something
> undesirable with it.  I think this possibility is remote but it should
> be mentioned in the commit message.

Note that this commit _does not_ invent any new protocol at all. It merely
add support for the protocol that is used by additional consoles. The
backend side was implemented by qemu only before, now I add support for
it to xenconsoled.

This is about (additional) consoles living in
/local/domain/$DOMID/device/console/$DEVID, not the special-kind-of-thing
living in /local/domain/$DOMID/console. Is there any protocol specification
for it already anywhere? I can't see it xen/include/public/io/ as it's
for other device types - console.h have only struct xencons_interface
declaration without any comment. I can't find it in other places either.
If there is one, it should be added to docs/misc/console.txt and/or
xen/include/public/io/console.h. Otherwise I can add some basic spec to
docs/misc/console.txt.

> What about the possibility that one or the other end of the connection
> may be replaced by a different implementation, so that the peer
> appears to gain or lose support for `state' ?

Actually, I'm doing similar thing here ;) previously xenconsoled didn't
know anything about 'state' field and it was one of the reason it
couldn't handle multiple consoles per domain.

> I'll be able to review the code more effectively when there is a
> formal protocol spec to compare it to...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v9 16/18] xen: automatically create XenBlockDevice-s

[Anthony PERARD]

On Tue, Jan 08, 2019 at 02:49:01PM +, Paul Durrant wrote:
> This patch adds create and destroy function for XenBlockDevice-s so that
> they can be created automatically when the Xen toolstack instantiates a new
> PV backend via xenstore. When the XenBlockDevice is created this way it is
> also necessary to create a 'drive' which matches the configuration that the
> Xen toolstack has written into xenstore. This is done by formulating the
> parameters necessary for each 'blockdev' layer of the drive and then using
> qmp_blockdev_add() to create the layers. Also, for compatibility with the
> legacy 'xen_disk' implementation, an iothread is automatically created for
> the new XenBlockDevice. This, like the driver layers, will be destroyed
> after the XenBlockDevice is unrealized.
> 
> The legacy backend scan for 'qdisk' is removed by this patch, which makes
> the 'xen_disk' code is redundant. The code will be removed by a subsequent
> patch.
> 
> Signed-off-by: Paul Durrant 
> Signed-off-by: Anthony Perard 
> ---

> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index a7c37c185a..c6ec1d9543 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -7,12 +7,20 @@
...

> +static XenBlockDrive *xen_block_drive_create(const char *id,
> + const char *device_type,
> + QDict *opts, Error **errp)
> +{
...
> +Error *local_err = NULL;
...
> +if (!filename) {
> +error_setg(errp, "no filename");
> +goto done;
> +}
...
> +drive->node_name = xen_block_blockdev_add(drive->id, driver_layer,
> +  _err);
> +
> +done:
> +g_free(driver);
> +g_free(filename);
> +
> +if (local_err) {

When xen_block_blockdev_add failed, it sets local_err, but nothing after
sets errp. I'm going to add this while preparing the pull request:

error_propagate(errp, local_err);

Is this fine with you?

With that fix, I think the series is ready, so:
Reviewed-by: Anthony PERARD 

> +xen_block_drive_destroy(drive, NULL);
> +return NULL;
> +}
> +
> +return drive;
> +}

There is still the warning about using the 'file' driver when
'host_device' should be use, but I think we can try to fix that later.

Thanks,

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC v2 0/4] QEMU changes to do PVH boot

[Stefano Garzarella]

Hi Liam,

On Tue, Jan 8, 2019 at 3:47 PM Liam Merwick  wrote:
> QEMU sets the hvm_modlist_entry in load_linux() after the call to
> load_elfboot() and then qboot loads it in boot_pvh_from_fw_cfg()
>
> But the current PVH patches don't handle initrd (they have
> start_info.nr_modules == 1).

Looking in the linux kernel (arch/x86/platform/pvh/enlighten.c) I saw:
/* The first module is always ramdisk. */
if (pvh_start_info.nr_modules) {
struct hvm_modlist_entry *modaddr =
__va(pvh_start_info.modlist_paddr);
pvh_bootparams.hdr.ramdisk_image = modaddr->paddr;
pvh_bootparams.hdr.ramdisk_size = modaddr->size;
}

So, putting start_info.nr_modules = 1, means that the first
hvm_modlist_entry should have the ramdisk paddr and size. Is it
correct?


>
> During (or after) the call to load_elfboot() it looks like we'd need to
> do something like what load_multiboot() does below (along with the
> associated initialisation)
>
> 400 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
> 401 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
> 402 fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
> 403  sizeof(bootinfo));
>

In this case I think they used the FW_CFG_INITRD_* to pass bootinfo
varibales to the guest, maybe we could add something like what
linux_load() does:

/* load initrd */
if (initrd_filename) {
...
initrd_addr = (initrd_max-initrd_size) & ~4095;

fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
...
}

Then we can load the initrd in qboot or in the optionrom that I'm writing.

What do you think?

Thanks,
Stefano

> I'm checking to see if that has any implications for the kernel side.
>
> Regards,
> Liam
>


-- 
Stefano Garzarella
Red Hat

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change()

[Jan Beulich]

>>> On 09.01.19 at 11:41,  wrote:
> finish_type_change() returns a negative int on error, but the
> current code checks if ( !rc ).

For the purpose of determining the backporting scope (none here)
it would be nice if in such a case you could point out the commit
introducing the breakage.

> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
>  
>  rc = finish_type_change(hostp2m, first_gfn, max_nr);
>  
> -if ( !rc )
> +if ( rc < 0 )
>  goto out;
>  
>  #ifdef CONFIG_HVM
> @@ -1193,13 +1193,13 @@ int p2m_finish_type_change(struct domain *d,
>  rc = finish_type_change(altp2m, first_gfn, max_nr);
>  p2m_unlock(altp2m);
>  
> -if ( !rc )
> +if ( rc < 0 )
>  goto out;
>  }
>  }
>  #endif

May I ask that you also fix the other return value issue here: Either
we mean to indicate to the caller the 0/1 distinction (in which case
it should be correct, i.e. cumulative across all finish_type_change()
calls made here), which was the behavior before 0fb4b58c8b
("x86/altp2m: fix display frozen when switching to a new view early"),
or we mean to signal success to the caller only by handing back zero.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

On Wed, Jan 09, 2019 at 10:58:21AM +, Anthony PERARD wrote:
> On Thu, Nov 29, 2018 at 11:39:54AM +, Wei Liu wrote:
> > On Thu, Nov 29, 2018 at 11:31:41AM +, Anthony PERARD wrote:
> > > On Wed, Nov 28, 2018 at 05:43:33PM +, Wei Liu wrote:
> > > > OVMF has become dependent on OpenSSL, which it is included as a 
> > > > submodule.
> > > > Initialise submodules before building.
> > > > 
> > > > Signed-off-by: Wei Liu 
> > > > ---
> > > > This should fix the build breakage for OVMF branch in OSSTEST.
> > > > 
> > > > Cc: Anthony PERARD 
> > > > Cc: Ian Jackson 
> > > > ---
> > > >  tools/firmware/ovmf-makefile | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > > > index 2838744461..3de2fc0300 100644
> > > > --- a/tools/firmware/ovmf-makefile
> > > > +++ b/tools/firmware/ovmf-makefile
> > > > @@ -16,6 +16,7 @@ all: build
> > > >  
> > > >  .PHONY: build
> > > >  build:
> > > > +   $(GIT) submodule update --init --recursive
> > > > OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
> > > > cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
> > > >  
> > > 
> > > What about the release tarball? Do we includes OVMF in it?
> > 
> > Yes we do. But this should work because the Makefile is also shipped.
> > What does qemu-xen do regarding its submodules? OVMF should just follow
> > suite.
> 
> I just found out that the answer to my question was incorrect, we don't
> ship OVMF sources code within the Xen release tarball. So the patch is
> fine as it is, we don't have to fix `make src-tarball-release` at all.
> 
> > > Also, doesn't osstest needs some updates? I forgot if there is something
> > > to do when projects have submodules.
> > 
> > Yes there is some special arrangement for libvirt. Not sure what needs
> > to be done for OVMF since it is part of xen.git.
> 
> For osstest, it doesn't seems that anything needs to be done. osstest
> doesn't know that QEMU is going to clone submodules, and yet osstest
> doesn't complain. Your patch have the build system takes care of
> submodules, so that's should be similair to qemu, and osstest isn't
> going to complain.
> 
> I've tested and run a flight with this patch and upstream ovmf, and it
> works fine, the openssl repo gets cloned thanks to the patch, and the
> xen build succeed.

Thank you very much for doing the hard work!

> 
> So for the patch:
> Reviewed-by: Anthony PERARD 

Thanks. I plan to push this fix with your Rb to unblock ovmf branch.

Wei.

> 
> -- 
> Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 5/6] x86/AMD: Fix handling of FPU pointer on Zen hardware

[Jan Beulich]

>>> On 28.12.18 at 13:39,  wrote:
> AMD hardware before Zen doesn't safe/restore the FPU error pointers
> unless an unmasked FPU exception is pending.  Zen processors have a
> feature bit indicating that this (mis)behaviour no longer exists.
> 
> Express the common logic in terms of cpu_bug_fpu_err_ptr as Hygon
> processors (being Zen derivatives) won't inherit this behaviour.

While they've decided to change the behavior, I still don't think this
was to be considered a bug, as it was well documented for a long
time (albeit perhaps not from the beginning). I again question the
validity of cpu_bug_ as a prefix here.

> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -369,15 +369,13 @@ void xrstor(struct vcpu *v, uint64_t mask)
>  unsigned int faults, prev_faults;
>  
>  /*
> - * AMD CPUs don't save/restore FDP/FIP/FOP unless an exception
> + * Some CPUs don't save/restore FDP/FIP/FOP unless an exception
>   * is pending. Clear the x87 state here by setting it to fixed
>   * values. The hypervisor data segment can be sometimes 0 and
>   * sometimes new user value. Both should be ok. Use the FPU saved
>   * data block as a safe address because it should be in L1.
>   */
> -if ( (mask & ptr->xsave_hdr.xstate_bv & X86_XCR0_FP) &&
> - !(ptr->fpu_sse.fsw & ~ptr->fpu_sse.fcw & 0x003f) &&
> - boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
> +if ( cpu_bug_fpu_err_ptr )
>  asm volatile ( "fnclex\n\t"/* clear exceptions */
> "ffree %%st(7)\n\t" /* clear stack tag */
> "fildl %0"  /* load to clear state */

This change will result in clobbering FPU state when X86_CR0_FP
is not set in "mask & ptr->xsave_hdr.xstate_bv". I'm anyway
unconvinced that dropping the conditions is a good thing, even
less so in an unrelated patch (a dedicated patch would otherwise
also adjust the similar FXRSTOR code).

> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -237,6 +237,7 @@ XEN_CPUFEATURE(EFRO,  7*32+10) /*   APERF/MPERF 
> Read Only interface */
>  
>  /* AMD-defined CPU features, CPUID level 0x8008.ebx, word 8 */
>  XEN_CPUFEATURE(CLZERO,8*32+ 0) /*A  CLZERO instruction */
> +XEN_CPUFEATURE(XSAVEERRPTR,   8*32+ 2) /*A  (F)XSAVE Error pointers always 
> updated. */

Any particular reason why you don't derive from AMD's name
(RstrFpErrPtrs)? Also if their documentation is to be trusted,
then the comment is wrongly mentioning FXSAVE; it's
questionable whether mentioning XSAVE but not XRSTOR is a
good thing.

As a side note - this is an example of a CPUID bit which would
better not require white listing for guest exposure.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Thu, Nov 29, 2018 at 11:39:54AM +, Wei Liu wrote:
> On Thu, Nov 29, 2018 at 11:31:41AM +, Anthony PERARD wrote:
> > On Wed, Nov 28, 2018 at 05:43:33PM +, Wei Liu wrote:
> > > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > > Initialise submodules before building.
> > > 
> > > Signed-off-by: Wei Liu 
> > > ---
> > > This should fix the build breakage for OVMF branch in OSSTEST.
> > > 
> > > Cc: Anthony PERARD 
> > > Cc: Ian Jackson 
> > > ---
> > >  tools/firmware/ovmf-makefile | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > > index 2838744461..3de2fc0300 100644
> > > --- a/tools/firmware/ovmf-makefile
> > > +++ b/tools/firmware/ovmf-makefile
> > > @@ -16,6 +16,7 @@ all: build
> > >  
> > >  .PHONY: build
> > >  build:
> > > + $(GIT) submodule update --init --recursive
> > >   OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
> > >   cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
> > >  
> > 
> > What about the release tarball? Do we includes OVMF in it?
> 
> Yes we do. But this should work because the Makefile is also shipped.
> What does qemu-xen do regarding its submodules? OVMF should just follow
> suite.

I just found out that the answer to my question was incorrect, we don't
ship OVMF sources code within the Xen release tarball. So the patch is
fine as it is, we don't have to fix `make src-tarball-release` at all.

> > Also, doesn't osstest needs some updates? I forgot if there is something
> > to do when projects have submodules.
> 
> Yes there is some special arrangement for libvirt. Not sure what needs
> to be done for OVMF since it is part of xen.git.

For osstest, it doesn't seems that anything needs to be done. osstest
doesn't know that QEMU is going to clone submodules, and yet osstest
doesn't complain. Your patch have the build system takes care of
submodules, so that's should be similair to qemu, and osstest isn't
going to complain.

I've tested and run a flight with this patch and upstream ovmf, and it
works fine, the openssl repo gets cloned thanks to the patch, and the
xen build succeed.

So for the patch:
Reviewed-by: Anthony PERARD 

-- 
Anthony PERARD

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH for-4.12] x86/p2m: fix p2m_finish_type_change()

[Razvan Cojocaru]

finish_type_change() returns a negative int on error, but the
current code checks if ( !rc ). Also properly indent the out:
label while at it.

Signed-off-by: Razvan Cojocaru 
---
 xen/arch/x86/mm/p2m.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 5451f16..e08f6b1 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1176,7 +1176,7 @@ int p2m_finish_type_change(struct domain *d,
 
 rc = finish_type_change(hostp2m, first_gfn, max_nr);
 
-if ( !rc )
+if ( rc < 0 )
 goto out;
 
 #ifdef CONFIG_HVM
@@ -1193,13 +1193,13 @@ int p2m_finish_type_change(struct domain *d,
 rc = finish_type_change(altp2m, first_gfn, max_nr);
 p2m_unlock(altp2m);
 
-if ( !rc )
+if ( rc < 0 )
 goto out;
 }
 }
 #endif
 
-out:
+ out:
 p2m_unlock(hostp2m);
 
 return rc;
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 4/6] x86/AMD: Introduce and use X86_BUG_NULL_SEG

[Jan Beulich]

>>> On 28.12.18 at 13:39,  wrote:
> AMD processors don't clear the base or limit fields when loading a NULL
> segment, and Hygon processors inherit this behaviour.
> 
> Express the logic in terms of cpu_bug_null_seg,

If this behavior was considered a bug, AMD surely would have fixed
it by now. It's a quirk at best, but personally I would call it just an
implementation detail. Hence cpu_bug_ as a prefix is simply
inappropriate.

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1307,16 +1307,16 @@ arch_do_vcpu_op(
>  }
>  
>  /*
> - * Loading a nul selector does not clear bases and limits on AMD CPUs. Be on
> - * the safe side and re-initialize both to flat segment values before loading
> - * a nul selector.
> + * Loading a NULL selector doesn't always clear bases and limits.  Be on the
> + * safe side and re-initialize both to flat segment values before loading a
> + * NULL selector.

To be honest I dislike the abuse of NULL in cases like this one: In
C code NULL would better stand for just the one thing the language
assigns to it as a meaning. Hence "nul" was better as a term (or if
you dislike its spelling, "null" would still be better).

>   */
> -#define preload_segment(seg, value) do {  \
> -if ( !((value) & ~3) &&   \
> - boot_cpu_data.x86_vendor == X86_VENDOR_AMD ) \
> -asm volatile ( "movl %k0, %%" #seg\
> -   :: "r" (FLAT_USER_DS32) ); \
> -} while ( false )
> +#define preload_segment(seg, value) \
> +do {\
> +if ( cpu_bug_null_seg && !((value) & ~3) )  \
> +asm volatile ( "mov %k0, %%" #seg   \
> +   :: "rm" (FLAT_USER_DS32) );  \

As you say in the description, "mov %sreg" allows for a memory
operand, so from an abstract perspective the change is fine. It
won't have any practical effect though (and hence be potentially
misleading), as "m" can't be combined with a literal number as
value (you may want to try out removing the r from the
constraint) - for that to work the compiler would have to allocate
an unnamed stack variable, which it doesn't do (instead you get
"memory input  is not directly addressable").

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 3/6] x86/AMD: Rework XSA-9 / Erratum 121 handling entirely

[Jan Beulich]

>>> On 28.12.18 at 13:39,  wrote:
> There are multiple problems:
> 
>  * The opt_allow_unsafe < 0 logic is dead since 2012 (c/s 0c7a6966511
>"x86-64: refine the XSA-9 fix").

Not really, no, the more that said commit only introduced it. Please
pay attention to the description saying "by means of a single line
change" as well as "don't default to boot denial". The thing I agree
was not possible (no idea how I did overlook it) was to set the
value to -1 without it defaulting to it.

>  * Given that opt_allow_unsafe was deliberately intended not to be
>specific to #121 alone, setting it to true for the not-affected case
>will cause a security issue if a second use of this option ever
>appears.
>  * Calling cpu_has_amd_erratum() on every domain creation is wasteful,
>given that the answer is static after boot.

Hardly a big performance impact, I would say.

> Move opt_allow_unsafe into domain.c, as a better location for it to
> live, and switch it to be a straight boolean.

So you think boot denial is not useful, and hence not worthwhile to
retain (and make properly command line controllable)?

> @@ -538,6 +534,14 @@ static void init_amd_k8(struct cpuinfo_x86 *c)
>  {
>  uint64_t val;
>  
> +setup_force_cpu_cap(X86_BUG_AMD_ERRATUM_121);
> +printk(KERN_WARNING
> +   "*** Xen will not allow DomU creation on this CPU for 
> security reasons ***\n"
> +   KERN_WARNING
> +   "*** Pass \"allow_unsafe\" if you trust all your guest 
> kernels ***\n");

This would affect all of Fam0F, while models 0x40 and higher aren't
affected from all I know.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [ovmf bisection] complete build-i386-xsm

[osstest service owner]

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: 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:  d2f1f6423bd1d23cf2fe77d7401961d771097369
  Bug not present: 5f1371270ec2e26367ff89b8b312f08c6adeb455
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/131878/


  commit d2f1f6423bd1d23cf2fe77d7401961d771097369
  Author: Fu Siyuan 
  Date:   Tue Oct 30 14:49:42 2018 +0800
  
  OvmfPkg: Replace obsoleted network drivers from platform DSC/FDF.
  
  V2:
  Add missed library instance for NetworkPkg iSCSI driver.
  
  This patch replaces the MdeModulePkg TCP, PXE and iSCSI driver with those
  ones in NetworkPkg. These 3 drivers in MdeModulePkg are not being actively
  maintained and will be removed from edk2 master soon.
  
  Cc: Jordan Justen 
  Cc: Laszlo Ersek 
  Cc: Ard Biesheuvel 
  Cc: Anthony Perard 
  Cc: Julien Grall 
  Contributed-under: TianoCore Contribution Agreement 1.1
  Signed-off-by: Fu Siyuan 
  Reviewed-by: Laszlo Ersek 


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/131878.bisection-summary --basis-template=129475 
--blessings=real,real-bisect ovmf build-i386-xsm xen-build
Searching for failure / basis pass:
 131846 fail [host=debina0] / 129475 [host=pinot1] 129454 [host=chardonnay0] 
129430 [host=baroque0] 129328 [host=albana1] 129310 [host=debina1] 129273 
[host=debina1] 129268 [host=baroque1] 129238 [host=baroque0] 129218 
[host=baroque1] 129212 [host=pinot0] 129191 [host=fiano0] 129171 [host=pinot0] 
129162 [host=albana0] 129125 ok.
Failure / basis pass flights: 131846 / 129125
(tree with no url: minios)
(tree with no url: seabios)
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: xen git://xenbits.xen.org/xen.git
Latest a53a888de8f5fa8dbf75a381e28f25a5497572f1 
d0d8ad39ecb51cd7497cd524484fe09f50876798 
de5b678ca4dcdfa83e322491d478d66df56c1986 
a5b0eb363694e7e15405f0b3fc5fb6fab79df1db
Basis pass c09b254bdc6050cc8b580a26558f692f958645d6 
9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 
de5b678ca4dcdfa83e322491d478d66df56c1986 
92666fdd6e0afab989b2d89759d9b43f2c645ae7
Generating revisions with ./adhoc-revtuple-generator  
https://github.com/tianocore/edk2.git#c09b254bdc6050cc8b580a26558f692f958645d6-a53a888de8f5fa8dbf75a381e28f25a5497572f1
 
git://xenbits.xen.org/qemu-xen-traditional.git#9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149-d0d8ad39ecb51cd7497cd524484fe09f50876798
 
git://xenbits.xen.org/qemu-xen.git#de5b678ca4dcdfa83e322491d478d66df56c1986-de5b678ca4dcdfa83e322491d478d66df56c1986
 
git://xenbits.xen.org/xen.git#92666fdd6e0afab989b2d89759d9b43f2c645ae7-a5b0eb363694e7e15405f0b3fc5fb6fab79df1db
Loaded 3004 nodes in revision graph
Searching for test results:
 129121 [host=albana0]
 129125 pass c09b254bdc6050cc8b580a26558f692f958645d6 
9c0eed618f37dd5b4a57c8b3fbc48ef8913e3149 
de5b678ca4dcdfa83e322491d478d66df56c1986 
92666fdd6e0afab989b2d89759d9b43f2c645ae7
 129191 [host=fiano0]
 129162 [host=albana0]
 129171 [host=pinot0]
 129212 [host=pinot0]
 129218 [host=baroque1]
 129238 [host=baroque0]
 129273 [host=debina1]
 129268 [host=baroque1]
 129310 [host=debina1]
 129328 [host=albana1]
 129430 [host=baroque0]
 129535 [host=debina1]
 129475 [host=pinot1]
 129454 [host=chardonnay0]
 129526 fail irrelevant
 129573 [host=debina1]
 129657 [host=debina1]
 129663 [host=debina1]
 129658 [host=debina1]
 129641 [host=debina1]
 129604 [host=debina1]
 129659 [host=debina1]
 129665 [host=debina1]
 129666 [host=debina1]
 129668 [host=debina1]
 129669 [host=debina1]
 129670 [host=debina1]
 129742 fail irrelevant
 129671 [host=debina1]
 129696 fail irrelevant
 129673 [host=debina1]
 129730 fail irrelevant
 129674 [host=debina1]
 129662 [host=debina1]
 129678 [host=debina1]
 129718 fail irrelevant
 129749 [host=debina1]
 129682 fail irrelevant
 129684 [host=debina1]
 129686 [host=debina1]
 129722 [host=debina1]
 129775 fail irrelevant
 129689 [host=debina1]
 129703 fail irrelevant
 129725 [host=debina1]
 129715 fail irrelevant
 129745 fail irrelevant
 129734 fail irrelevant
 129753 fail irrelevant
 129764 fail irrelevant
 129756 fail irrelevant
 129769 [host=debina1]
 129808 fail irrelevant
 129813 [host=debina1]
 129832 fail irrelevant
 129816 [host=debina1]
 129821 fail irrelevant
 129797 [host=debina1]
 

[Xen-devel] [xen-unstable-coverity test] 131875: all pass - PUSHED

[osstest service owner]

flight 131875 xen-unstable-coverity real [real]
http://logs.test-lab.xenproject.org/osstest/logs/131875/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xen  1292f9a05943d32ef96eabb9f0c30cf681665c46
baseline version:
 xen  a5b0eb363694e7e15405f0b3fc5fb6fab79df1db

Last test of basis   131768  2019-01-06 09:18:24 Z3 days
Testing same since   131875  2019-01-09 09:19:00 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Roger Pau Monné 
  Stefano Stabellini 
  Stefano Stabellini 

jobs:
 coverity-amd64   pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   a5b0eb3636..1292f9a059  1292f9a05943d32ef96eabb9f0c30cf681665c46 -> 
coverity-tested/smoke

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 2/2] xen: use SYMBOL when required

[Jan Beulich]

>>> On 08.01.19 at 19:08,  wrote:
> On Tue, 8 Jan 2019, Stefano Stabellini wrote:
>> So, this is what I am going to do: I'll send a series update according
>> to your suggestion, with SYMBOL returning the native pointer type. As I
>> wrote earlier, although weaker, it is still an improvement from my point
>> of view.
> 
> There is a problem with this though I didn't foresee :-(
> 
> The native type of _start is not char* -- it is char[]. So I cannot
> actually return the native type from SYMBOL because I cannot cast to
> (char[]). I didn't notice it until I actually tried it.
> 
> See the implementation of RELOC_HIDE:
> 
>   #define RELOC_HIDE(ptr, off)\
> ({ unsigned long __ptr;   \
>   __asm__ ("" : "=r"(__ptr) : "0"(ptr));  \
>   (typeof(ptr)) (__ptr + (off)); })
> 
> It casts to the type at the end, the error is:
> 
>   error: cast specifies array type
>(typeof(ptr)) (__ptr + (off)); })
> 
> We have a few options:
> 
> 1) use unsigned long as in this version of the series (the Linux kernel
> also uses this technique)
> Sorry if I insist, it is still the best I think :-)
> 
> 2) casts the parameters of SYMBOL to the corresponding pointer type
> For instance:
>   SYMBOL((char *)_start)
>   SYMBOL((struct alt_instr *)__alt_instructions_end)
> This works, but it is ugly, I would say it makes the code worse than
> option 1)
> 
> 2) always return void* from SYMBOL
> I don't think it is a good idea to use void* as a workaround here
> 
> 3) pass the desired return type to SYMBOL
> For instance:
>   SYMBOL(_start, char *)
>   SYMBOL(__alt_instructions_end, struct alt_instr *)
> Then SYMBOL would automatically cast the return type to char * and
> struct alt_instr * according to the second parameter.
> 
> Do you have any other suggestions?

4) 

#define RELOC_HIDE(ptr, off)\
({ unsigned long ptr_;   \
  __asm__ ("" : "=r"(ptr_) : "0" (ptr));  \
  (typeof(*(ptr)) *) (ptr_ + (off)); })

or, if not suitable for RELOC_HIDE() itself, clone the macro into one
that fits SYMBOL()'s needs.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v3 04/15] argo: init, destroy and soft-reset, with enable command line opt

[Jan Beulich]

>>> On 08.01.19 at 23:54,  wrote:

First of all - please trim your replies.

> On Mon, Jan 7, 2019 at 2:43 AM Christopher Clark 
>  wrote:
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -182,6 +182,17 @@ Permit Xen to use "Always Running APIC Timer" support 
>> on compatible hardware
>>  in combination with cpuidle.  This option is only expected to be useful for
>>  developers wishing Xen to fall back to older timing methods on newer 
>> hardware.
>>
>> +### argo
>> +> `= `
>> +
>> +> Default: `false`
>> +
>> +Enable the Argo hypervisor-mediated interdomain communication mechanism.
>> +
>> +This allows domains access to the Argo hypercall, which supports 
>> registration
>> +of memory rings with the hypervisor to receive messages, sending messages to
>> +other domains by hypercall and querying the ring status of other domains.
>> +
> 
> Do we want to say it's only available when Xen is compiled with CONFIG_ARGO?

We don't do so elsewhere, so I'm with Christopher.

>> + */
>> +struct argo_ring_info *ring_info;
>> +/* domain to be notified when space is available */
>> +domid_t domain_id;
>> +uint16_t pad;
> 
> Can we order domain_id after len and drop the pad?

That would still call for a pad field - we prefer to have explicit padding,
and also to check it's zero, the latter to allow for assigning meaning to
the field down the road.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC PATCH 2/2] x86/mm: Add mem access rights to NPT

[Alexandru Stefan ISAILA]

Ping

Suravee / Brian / Boris any ideas on this topic are appreciated.

Regards,
Alex

On 27.09.2018 13:37, George Dunlap wrote:
> On 09/26/2018 06:22 PM, Andrew Cooper wrote:
>> On 26/09/18 17:47, George Dunlap wrote:
>>> From: Isaila Alexandru 
>>>
>>> This patch adds access control for NPT mode.
>>>
>>> There aren’t enough extra bits to store the access rights in the NPT p2m
>>> table, so we add a radix tree to store extra information.
>>
>> I'm sorry to re-open this argument, but why?
>>
>> ISTR there being some argument based on pagetable sharing with the
>> IOMMU, but that doesn't work at the moment and can't reasonably be made
>> to work.  For one, attempting to use pt sharing will break as soon as
>> you try and DMA to a mapped grant.
>>
>> I'm disinclined to let a broken vestigial feature get in the way of real
>> improvements.
>>
>> Beyond that, an NPT PTE has basically the same number of software
>> available bits as an EPT PTE.
>>
>> Am I missing anything?
> 
> Wow -- looks like IOMMU/p2m sharing has been disabled unconditionally
> since 2014.  If nobody has complained since then, that seems like a good
> enough reason to me to rip it out.
> 
> Suravee / Brian / Boris -- any opinions?
> 
> The main reason to go with the 'extra bits' solution rather than the
> 'rip out iommu/p2m sharing' solution is because people have been
> prognosticating for years that we would be running out of bits and need
> more at some point in the future.  I thought Paul, for instance, might
> have a use for the extra bits.  But I'm happy to wait until such time as
> we need it and then fish this patch out of the mail archives.
> 
>   -George
> 
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN][ARM64]: Qemu unhandled level 1 translation fault and p2m failure

[vikram k.s.]

Hello,

   - We are using XEN-4.12.
   - Linux 4.14 as dom0 and Linux4.19 as domU.

When domU is create getting below log.
qemu-system-i38[3478]: unhandled level 1 translation fault (11) at
0x0010, esr 0x9205, in libxengnttab.so.1.2[7faf9cd000+3000]

Also  p2m is failing .
(XEN) p2m.c:1456: d2v4: gvirt_to_maddr failed va=0x80001dfa1090
flags=0x1 par=0x80

Not sure whether these two issue are dependent or not. Please help in
resolving this issue.

Please find the complete domU boot log attached below.

Regards
Vks
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [RFC PATCH 6/6] xc_version: add vm_event interface version

[Razvan Cojocaru]

On 1/8/19 6:47 PM, Jan Beulich wrote:

On 08.01.19 at 17:37,  wrote:

On 1/8/19 6:27 PM, Jan Beulich wrote:

On 19.12.18 at 19:52,  wrote:

Signed-off-by: Petre Pircalabu 


An empty description is not helpful. The immediate question is: Why?
We don't do this for other interface versions. I'm unconvinced a
special purpose piece of information like this one belongs into the
rather generic version hypercall.


For an introspection application meant to be deployed on several Xen
versions without recompiling, it is important to be able to decide at
runtime what size and layout the vm_event struct has.

Currently this can somewhat be done by associating the current version
with the vm_event version, but that is not ideal for obvious reasons.
Reading the vm_event version from an actual vm_event is also out of the
question, because in order to be able to receive the first vm_event we
have to set the ring buffer up, and that requires knowledge of the size
of the vm_event. So a run-time mechanism for querying the vm_event
version is needed.

We just thought that this was the most flexible place to add it.


How about a new XEN_DOMCTL_VM_EVENT_GET_VERSION?


That would work as well, we just thought this was the least intrusive 
and most extensible way to do it (other queries could be added similarly 
in the future, without needing a new DOMCTL / libxc toolstack 
modifications).



Thanks,
Razvan

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] tmem: default to off

[Juergen Gross]

On 09/01/2019 10:05, Jan Beulich wrote:
> As a short term alternative to deleting the code, default its building
> to off (overridable in EXPERT mode only). Additionally make sure other
> related baggage (LZO code) won't be carried when the option is off (with
> TMEM scheduled to be deleted anyway, I didn't want to introduce a
> separate Kconfig option to control the LZO compression code, and hence
> CONFIG_TMEM is used directly there). Similarly I couldn't be bothered to
> add actual content to the command line option doc for the two affected
> options.
> 
> Signed-off-by: Jan Beulich 
> ---
> While not posted previously, this is meant to be considered a
> replacement for the TMEM removal patch, which was posted in time. I
> therefore would like to see this allowed in for 4.12.

Release-acked-by: Juergen Gross 


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH] tmem: default to off

[Jan Beulich]

As a short term alternative to deleting the code, default its building
to off (overridable in EXPERT mode only). Additionally make sure other
related baggage (LZO code) won't be carried when the option is off (with
TMEM scheduled to be deleted anyway, I didn't want to introduce a
separate Kconfig option to control the LZO compression code, and hence
CONFIG_TMEM is used directly there). Similarly I couldn't be bothered to
add actual content to the command line option doc for the two affected
options.

Signed-off-by: Jan Beulich 
---
While not posted previously, this is meant to be considered a
replacement for the TMEM removal patch, which was posted in time. I
therefore would like to see this allowed in for 4.12.

--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1969,9 +1969,15 @@ pages) must also be specified via the tb
 ### tmem
 > `= `
 
+This option (and its underlying code) is going to go away in a future
+Xen version.
+
 ### tmem_compress
 > `= `
 
+This option (and its underlying code) is going to go away in a future
+Xen version.
+
 ### tsc (x86)
 > `= unstable | skewed | stable:socket`
 
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -78,17 +78,19 @@ config KEXEC
  If unsure, say Y.
 
 config TMEM
-   def_bool y
-   prompt "Transcendent Memory Support" if EXPERT = "y"
+   bool "Transcendent Memory Support (deprecated)" if EXPERT = "y"
---help---
  Transcendent memory allows PV-aware guests to collaborate on memory
  usage. Guests can 'swap' their memory to the hypervisor or have an
  collective pool of memory shared across guests. The end result is
  less memory usage by guests allowing higher guest density.
 
- You also have to enable it on the Xen commandline by using tmem=1
+ You also have to enable it on the Xen commandline by using tmem=1.
 
- If unsure, say Y.
+ WARNING: This option (and its underlying code) is going to go away
+ in a future Xen version.
+
+ If unsure, say N.
 
 config XENOPROF
def_bool y
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -21,7 +21,7 @@ obj-$(CONFIG_KEXEC) += kimage.o
 obj-y += lib.o
 obj-$(CONFIG_NEEDS_LIST_SORT) += list_sort.o
 obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
-obj-y += lzo.o
+obj-$(CONFIG_TMEM) += lzo.o
 obj-$(CONFIG_MEM_ACCESS) += mem_access.o
 obj-y += memory.o
 obj-y += monitor.o
@@ -66,8 +66,9 @@ obj-bin-y += warning.init.o
 obj-$(CONFIG_XENOPROF) += xenoprof.o
 obj-y += xmalloc_tlsf.o
 
-obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo 
unlz4 earlycpio,$(n).init.o)
-
+lzo-y := lzo
+lzo-$(CONFIG_TMEM) :=
+obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma $(lzo-y) 
unlzo unlz4 earlycpio,$(n).init.o)
 
 obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o 
multicall.o xlat.o)
 
--- a/xen/common/lzo.c
+++ b/xen/common/lzo.c
@@ -105,6 +105,8 @@
 #define get_unaligned_le16(_p) (*(u16 *)(_p))
 #define get_unaligned_le32(_p) (*(u32 *)(_p))
 
+#ifdef CONFIG_TMEM
+
 static noinline size_t
 lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
 unsigned char *out, size_t *out_len,
@@ -362,6 +364,11 @@ int lzo1x_1_compress(const unsigned char
 return LZO_E_OK;
 }
 
+# define INIT
+#else /* CONFIG_TMEM */
+# include "decompress.h"
+#endif /* CONFIG_TMEM */
+
 /*
  *  LZO1X Decompressor from LZO
  *
@@ -391,8 +398,8 @@ int lzo1x_1_compress(const unsigned char
  */
 #define MAX_255_COUNT  size_t)~0) / 255) - 2)
 
-int lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
-  unsigned char *out, size_t *out_len)
+int INIT lzo1x_decompress_safe(const unsigned char *in, size_t in_len,
+   unsigned char *out, size_t *out_len)
 {
 unsigned char *op;
 const unsigned char *ip;





___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN][ARM64]: Qemu unhandled level 1 translation fault and p2m failure

[vikram k.s.]

Hello,

   - We are using XEN-4.12.
   - Linux 4.14 as dom0 and Linux4.19 as domU.

When domU is create getting below log.
qemu-system-i38[3478]: unhandled level 1 translation fault (11) at
0x0010, esr 0x9205, in libxengnttab.so.1.2[7faf9cd000+3000]

Also  p2m is failing .
(XEN) p2m.c:1456: d2v4: gvirt_to_maddr failed va=0x80001dfa1090
flags=0x1 par=0x80

Not sure whether these two issue are dependent or not. Please help in
resolving this issue.

Please find the complete domU boot log attached below.

Regards
VKS
[  282.887225] IPv6: ADDRCONF(NETDEV_UP): vif2.0: link is not ready
[  282.893866] IPv6: ADDRCONF(NETDEV_UP): vif2.0: link is not ready
[  282.923047] xenbr0: port 1(vif2.0) entered blocking state
[  282.928441] xenbr0: port 1(vif2.0) entered disabled state
[  282.934115] device vif2.0 entered promiscuous mode
[  282.941478] IPv6: ADDRCONF(NETDEV_UP): vif2.0: link is not ready
[  283.058188] qemu-system-i38[3478]: unhandled level 1 translation fault (11) 
at 0x0010, esr 0x9205, in libxengnttab.so.1.2[7faf9cd000+3000]
[  283.071564] CPU: 1 PID: 3478 Comm: qemu-system-i38 Tainted: G S  
4.14.0-rc7 #1
[  283.079993] Hardware name: HiKey960 (DT)
(XEN) d2v0: vGICD: unhandled word write 0x to ICACTIVER0
[  283.083907] task: ffc004a7d080 task.stack: ff800f4e8000
[  283.095771] PC is at 0x7faf9ce7ac
[  283.099156] LR is at 0x7faf9ce294
[  283.102426] pc : [<007faf9ce7ac>] lr : [<007faf9ce294>] pstate: 
8000
[  283.109918] sp : 007ff8ad26f0
[  283.113286] x29: 007ff8ad[0.00] Booting Linux on physical CPU 
0x00 [0x410fd034]
[0.00] Linux version 4.19.13 (vikram.k@CPU-248U26f0 x28: 00) (gcc 
version 7.3.1 20180425 [linaro-7.3-2018.05 revision 
d29120a424ecfbc167ef90065c0eeb7f91977701] (Linaro GCC 7.3-2018.05)) 00 
#1 SMP PREEMPT Thu Jan 3 11:31:48 IST 2019
[0.00] Machine model: XENVM-4.12
[0.00] Xen 4.12 support found
[   [  283.140924] x 0.00] efi: Getting EFI parameters from FDT:
[0.00] efi: UEFI not found.
[0.00] cma: Reserved 32 MiB at 027: 00556834x02b60 x26: 
05e00
[0.00] NUMA: No NUMA configuration found
[0.00] NUMA: Faking a node at [mem 0x0 
0-0x5fff]
[0.00] NUMA: NODE_DATA [mem 0x5dff-0x5dff17bf]
[0.00] Zone ranges:
[0.00] [  283.180230] x(XEN) d2v1: vGICD: unhandled word write 
0x to ICACTIVER0
  25: DMA32[mem 0x4000-0x5fff]
[0.00]   Normal   empty
[0.00] Movable zone start for ea x24: 00ch node
[0.00] Early memory node ranges
[0.00]   node   0: [mem 0x4000-0x5fff]
[00 
0.[  283.225218] x00] Initmem setup node 0 [mem 
0x4000-0x5fff]
[0.00] psci: probing for conduit method from 23: 007faf97(XEN) 
d2v2: vGICD: unhandled word write 0x to ICACTIVER0
DT.
[0.00] psci: PSCIv1.1 detected in firmware.
[0.00] psci: Using standard PSCI v0.2 function IDs
[0.00d000 x22: 55691e62f0 
00] psci: Trusted OS migration not required
[0.00] psci: SMC Calling Convention v1.1
[0.00] random: get_rando[  283.258952] xm_bytes called from 
start_kernel+0xa8/0x418 with crng_init=0
[0.00] percpu: Embedded 21 pages/cpu @(ptrval) s521: 
0055691d(XEN) d2v3: vGICD: unhandled word write 0x to ICACTIVER0
681e40 x20: 0056 r0 d29160 u86016
[0.00] Detected VIPT I-cache on CPU0
[0.00] CPU features: enabling workaround for ARM err00 
atum 845719
[0.00] Speculative Store Bypass Disable mitigation not required
[0.00] CPU features: detected: Ke[  283.303749] xrn19: 0055691fel 
page table isolation (KPTI)
[0.00] Built 1 zonelists, mobility grouping on.  Total pages: 129024
[0.00] Pof478 x18: 00(XEN) d2v4: vGICD: unhandled word write 
0x to ICACTIVER0
licy zone: DMA32
[0.00] Kernel command line: earlyprintk=xenboot console=hvc0 
root=/dev/xvda rootfstype=ext4 rw video=00 
HD[  283.348445] xMI-A-1:1280x720@60
[0.00] Memory: 446440K/524288K available (17212K kernel code, 1918K 
rwdata, 6684K rodata, 1536K ini17: 00556881t, 445K bss, 45080K reserved, 
32768K cma-reserved)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=8, Nodes=1
7a90 x16: 00(XEN) d2v5: vGICD: unhandled word write 0x to ICACTIVER0
[ 7faf9ce278 
   0.00] rcu: Preemptible hierarchical RCU implementation.
[0.00] rcu: RCU restricting CPUs from NR_CPUS=64 to nr[  
283.382180] x_cpu_ids=8.
[0.00]  Tasks RCU enabled.
[0.00] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=8
[15:   0026 x14: 00  0.00] NR_IRQS: 64, nr_irqs: 64, 
preallocated irqs: 0
[0.00] arch_timer: cp15 timer(s) running at 1.92MHz (virt).00