Re: [Xen-devel] [RFC PATCH 3/6] vm_event: Refactor vm_event_domain implementation

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, Dec 19, 2018 at 11:52 AM Petre Pircalabu
<ppircalabu@xxxxxxxxxxxxxxx> wrote:
>
> Decouple the VM Event interface from the ring implementation.

This will need a much better description. There is also a lot of churn
that is mostly just mechanical in this patch but makes reviewing it
hard. Perhaps functional changes and mechanical changes could be split
to two patches?

> +struct vm_event_domain
> +{
> +    /* VM event ops */
> +    bool (*check)(struct vm_event_domain *ved);
> +    int (*claim_slot)(struct vm_event_domain *ved, bool allow_sleep);
> +    void (*release_slot)(struct vm_event_domain *ved);
> +    void (*put_request)(struct vm_event_domain *ved, vm_event_request_t 
> *req);
> +    int (*get_response)(struct vm_event_domain *ved, vm_event_response_t 
> *rsp);
> +    int (*disable)(struct vm_event_domain **_ved);

I don't see (yet) the reason why having these pointers stored in the
struct is needed. Are there going to be different implementations for
these? If so, need to explain that in the commit message.

> +
> +    /* The domain associated with the VM event */
> +    struct domain *d;
> +
> +    /* ring lock */
> +    spinlock_t lock;
> +};
> +
> +bool vm_event_check(struct vm_event_domain *ved)
> +{
> +    return (ved && ved->check(ved));
> +}
>
> -    return rc;
> +/* VM event domain ring implementation */
> +struct vm_event_domain_ring
> +{
> +    /* VM event domain */
> +    struct vm_event_domain ved;

Why is this not a pointer instead? Does each vm_event_domain_ring
really need a separate copy of vm_event_domain?

> +    /* The ring has 64 entries */
> +    unsigned char foreign_producers;
> +    unsigned char target_producers;
> +    /* shared ring page */
> +    void *ring_page;
> +    struct page_info *ring_pg_struct;
> +    /* front-end ring */
> +    vm_event_front_ring_t front_ring;
> +    /* event channel port (vcpu0 only) */
> +    int xen_port;
> +    /* vm_event bit for vcpu->pause_flags */
> +    int pause_flag;
> +    /* list of vcpus waiting for room in the ring */
> +    struct waitqueue_head wq;
> +    /* the number of vCPUs blocked */
> +    unsigned int blocked;
> +    /* The last vcpu woken up */
> +    unsigned int last_vcpu_wake_up;
> +};
> +

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