Re: [Xen-devel] [PATCH RFC v1 57/74] x86/pv-shim: shadow PV console's page for L2 DomU

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 14:06, <wei.liu2@xxxxxxxxxx> wrote:
> @@ -125,13 +127,28 @@ void __init pv_shim_setup_dom(struct domain *d, 
> l4_pgentry_t *l4start,
>  })
>      SET_AND_MAP_PARAM(HVM_PARAM_STORE_PFN, si->store_mfn, store_va);
>      SET_AND_MAP_PARAM(HVM_PARAM_STORE_EVTCHN, si->store_evtchn, 0);
> +    SET_AND_MAP_PARAM(HVM_PARAM_CONSOLE_EVTCHN, si->console.domU.evtchn, 0);
>      if ( !pv_console )
> -    {
>          SET_AND_MAP_PARAM(HVM_PARAM_CONSOLE_PFN, si->console.domU.mfn,
>                            console_va);
> -        SET_AND_MAP_PARAM(HVM_PARAM_CONSOLE_EVTCHN, si->console.domU.evtchn, 
> 0);
> -    }
>  #undef SET_AND_MAP_PARAM
> +    else
> +    {
> +        /* Allocate a new page for DomU's PV console */
> +        void *page = alloc_xenheap_pages(0, MEMF_bits(32));
> +        uint64_t console_mfn;
> +
> +        ASSERT(page);
> +        clear_page(page);
> +        console_mfn = virt_to_mfn(page);
> +        si->console.domU.mfn = console_mfn;
> +        share_xen_page_with_guest(mfn_to_page(console_mfn), d,
> +                                  XENSHARE_writable);
> +        replace_va(d, l4start, console_va, console_mfn);
> +        dom0_update_physmap(d, (console_va - va_start) >> PAGE_SHIFT,
> +                            console_mfn, vphysmap);
> +        consoled_set_ring_addr(page);

This looks to be a fair part of SET_AND_MAP_PARAM(), so I think
this wants breaking out as a separate macro.


> +size_t consoled_guest_rx(void)
> +{
> +    size_t recv = 0, idx = 0;
> +    XENCONS_RING_IDX cons, prod;
> +
> +    if ( !cons_ring )
> +        return 0;
> +
> +    spin_lock(&rx_lock);
> +
> +    cons = cons_ring->out_cons;
> +    prod = ACCESS_ONCE(cons_ring->out_prod);
> +    ASSERT((prod - cons) <= sizeof(cons_ring->out));
> +
> +    /* Is the ring empty? */
> +    if ( cons == prod )
> +        goto out;
> +
> +    /* Update pointers before accessing the ring */
> +    smp_rmb();

I think this need to move up ahead of the if(). In the comment
perhaps s/Update/Latch/?

> +size_t consoled_guest_tx(char c)
> +{
> +    size_t sent = 0;
> +    XENCONS_RING_IDX cons, prod;
> +
> +    if ( !cons_ring )
> +        return 0;
> +
> +    cons = ACCESS_ONCE(cons_ring->in_cons);
> +    prod = cons_ring->in_prod;
> +    ASSERT((prod - cons) <= sizeof(cons_ring->in));
> +
> +    /* Is the ring out of space? */
> +    if ( sizeof(cons_ring->in) - (prod - cons) == 0 )
> +        goto notify;
> +
> +    /* Update pointers before accessing the ring */
> +    smp_rmb();

Same here.

> --- /dev/null
> +++ b/xen/include/xen/consoled.h
> @@ -0,0 +1,27 @@
> +#ifndef __XEN_CONSOLED_H__
> +#define __XEN_CONSOLED_H__
> +
> +#include <public/io/console.h>
> +
> +#ifdef CONFIG_PV_SHIM
> +
> +void consoled_set_ring_addr(struct xencons_interface *ring);
> +struct xencons_interface *consoled_get_ring_addr(void);
> +size_t consoled_guest_rx(void);
> +size_t consoled_guest_tx(char c);
> +
> +#else
> +
> +size_t consoled_guest_tx(char c) { return 0; }

static inline

Jan


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