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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-04-11 at 15:38 +0100, Daniel De Graaf wrote:
> On 04/11/2013 10:14 AM, Ian Campbell wrote:
> > 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 frontend's write to pg->state is always done prior to the frontend
> sending its event channel notification, so an explicit barrier is not
> needed in this function.

An event channel notification might happen to include barriers as part
of its implementation but does the interface make any guarantees?

>  Since there is only one read and a clear
> dependency on the one write, so I'm not sure where the barrier here
> would need to go even if it was needed.

DOMU                            DOM_TPM
        PRE: pg->status == IDLE
        write data
        barrier()
        write pg->status = SUBMIT (A)
        notify evtchn

                                receive evtchn
                                read pg->status (B)
                                do some stuff with the data
                                write pg->status (C)

Don't we need some sort of memory barrier (as in rmb/wmb/mb not a
compiler barrier()) between (A) and (B), to ensure that B sees the write
at A and gets SUBMIT and not the previous value of IDLE?

I wasn't thinking about barriers between the read at (B) and the write
at (C), although now that you mention it a barrier might be needed
*after* (C) so that the domU sees the vTPM as IDLE next time it come to
use it...

Ian.



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