Re: [Xen-devel] [PATCH v5.1 01/12] mini-os/tpm{back, front}: Change shared page ABI

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-03-22 at 22:30 +0000, Daniel De Graaf wrote:
> @@ -529,15 +526,27 @@ void free_tpmif(tpmif_t* tpmif)
>  void tpmback_handler(evtchn_port_t port, struct pt_regs *regs, void *data)
>  {
>     tpmif_t* tpmif = (tpmif_t*) data;
> -   tpmif_tx_request_t* tx = &tpmif->tx->ring[0].req;
> -   /* Throw away 0 size events, these can trigger from event channel 
> unmasking */
> -   if(tx->size == 0)
> -      return;
> -
> -   TPMBACK_DEBUG("EVENT CHANNEL FIRE %u/%u\n", (unsigned int) tpmif->domid, 
> tpmif->handle);
> -   tpmif_req_ready(tpmif);
> -   wake_up(&waitq);
> +   vtpm_shared_page_t* pg = tpmif->page;
> 

Do we not need a barrier somewhere around here to ensure that the far
end's write to pg->state is visible to this cpu?

The writer does:
        write all fields apart from state
        barrier()
        write state.

no need for a barrier at the end of that lot either?

> +   switch (pg->state)
> +   {
> +   case VTPM_STATE_SUBMIT:
> +      TPMBACK_DEBUG("EVENT CHANNEL FIRE %u/%u\n", (unsigned int) 
> tpmif->domid, tpmif->handle);
> +      tpmif_req_ready(tpmif);
> +      wake_up(&waitq);
> +      break;
> +   case VTPM_STATE_CANCEL:
> +      /* If we are busy with a request, do nothing */
> +      if (tpmif->flags & TPMIF_REQ_READY)
> +         return;
> +      /* Acknowledge the cancellation if we are idle */
> +      pg->state = VTPM_STATE_IDLE;
> +      notify_remote_via_evtchn(tpmif->evtchn);
> +      return;
> +   default:
> +      /* Spurious wakeup; do nothing */
> +      return;
> +   }
>  }
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel