From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH v5.1 01/12] mini-os/tpm{back, front}: Change shared page ABI Date: Thu, 11 Apr 2013 11:46:53 -0400 Message-ID: <5166DAED.4040005@tycho.nsa.gov> References: <1363991313-25481-1-git-send-email-dgdegra@tycho.nsa.gov> <1363991420-25569-1-git-send-email-dgdegra@tycho.nsa.gov> <1365689654.8126.4.camel@zakaz.uk.xensource.com> <5166CAD4.7090209@tycho.nsa.gov> <1365693945.8126.42.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1365693945.8126.42.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "Matthew.Fioravante@jhuapl.edu" , "xen-devel@lists.xen.org" , Tim Deegan , "JBeulich@suse.com" , "konrad.wilk@oracle.com" List-Id: xen-devel@lists.xenproject.org On 04/11/2013 11:25 AM, Ian Campbell wrote: > 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 it looks like other mini-os drivers use wmb() before their event channel notifications, I'll add that to be consistent. >> 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. -- Daniel De Graaf National Security Agency