|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 08/18] OvmfPkg/XenbusDxe: Add Grant Table functions.
On Wed, Jul 16, 2014 at 01:48:22PM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 16, 2014 at 04:15:37PM +0100, Anthony PERARD wrote:
> > There are used to grant access of pages to other Xen domains.
> >
> > This code originaly comes from the Xen Project, and more precisely from
> > MiniOS.
>
> Should you mention something about its license?
Just in case, maybe. But both mini-os and ovmf are mostly BSD, so it's
probably not a big issue either way. I'll make sure to better describe
the original licenses in the commit messages.
> >
> > Signed-off-by: Steven Smith <sos22@xxxxxxxxx>
> > Signed-off-by: Grzegorz Milos <gm281@xxxxxxxxx>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> > OvmfPkg/XenbusDxe/GrantTable.c | 204
> > ++++++++++++++++++++++++++++++++++++++++
> > OvmfPkg/XenbusDxe/GrantTable.h | 34 +++++++
> > OvmfPkg/XenbusDxe/XenbusDxe.c | 15 +++
> > OvmfPkg/XenbusDxe/XenbusDxe.inf | 2 +
> > 4 files changed, 255 insertions(+)
> > create mode 100644 OvmfPkg/XenbusDxe/GrantTable.c
> > create mode 100644 OvmfPkg/XenbusDxe/GrantTable.h
> >
> > diff --git a/OvmfPkg/XenbusDxe/GrantTable.c b/OvmfPkg/XenbusDxe/GrantTable.c
> > new file mode 100644
> > index 0000000..97bd15a
> > --- /dev/null
> > +++ b/OvmfPkg/XenbusDxe/GrantTable.c
> > @@ -0,0 +1,204 @@
> > +/*
> > +
> > ****************************************************************************
> > + * (C) 2006 - Cambridge University
> > +
> > ****************************************************************************
> > + *
> > + * File: GrantTable.c
> > + * Author: Steven Smith (sos22@xxxxxxxxx)
> > + * Changes: Grzegorz Milos (gm281@xxxxxxxxx)
> > + *
> > + * Date: July 2006
> > + *
> > + * Environment: Xen Minimal OS
> > + * Description: Simple grant tables implementation. About as stupid as it's
> > + * possible to be and still work.
> > + *
> > +
> > ****************************************************************************
> > + */
> > +#include "XenbusDxe.h"
> > +
> > +#include <IndustryStandard/Xen/memory.h>
> > +
> > +#include "XenHypercall.h"
> > +
> > +#include "GrantTable.h"
> > +#include "InterlockedCompareExchange16.h"
> > +
> > +#define NR_RESERVED_ENTRIES 8
> > +
> > +/* NR_GRANT_FRAMES must be less than or equal to that configured in Xen */
> > +#define NR_GRANT_FRAMES 4
> > +#define NR_GRANT_ENTRIES (NR_GRANT_FRAMES * EFI_PAGE_SIZE /
> > sizeof(grant_entry_v1_t))
> > +
> > +STATIC grant_entry_v1_t *GrantTable = NULL;
> > +STATIC grant_ref_t GrantList[NR_GRANT_ENTRIES];
> > +#ifdef GNT_DEBUG
> > +STATIC BOOLEAN GrantInUseList[NR_GRANT_ENTRIES];
> > +#endif
> > +
> > +STATIC
> > +VOID
> > +XenGrantTablePutFreeEntry (
> > + grant_ref_t Ref
> > + )
> > +{
> > + //local_irq_save(flags);
> > + /* MemoryFence (); */
> > + // lock ?
>
> You just need some form of locking. If EFI has some simple Mutex() you can
> use that.
No, no mutex available, but there are some form of lock. It increase the
priority of the current task, so nothing else can run.
> > +#ifdef GNT_DEBUG
> > + ASSERT (GrantInUseList[Ref]);
> > + GrantInUseList[Ref] = FALSE;
> > +#endif
> > + GrantList[Ref] = GrantList[0];
> > + GrantList[0] = Ref;
> > + //local_irq_restore(flags);
> > + /* MemoryFence (); */
> > +}
> > +
> > +STATIC
> > +grant_ref_t
> > +XenGrantTableGetFreeEntry (
> > + VOID
> > + )
> > +{
> > + UINTN Ref;
> > + /* local_irq_save(flags); */
> > + // lock ??
> > + Ref = GrantList[0];
> > + ASSERT (Ref >= NR_RESERVED_ENTRIES && Ref < NR_GRANT_ENTRIES);
> > + GrantList[0] = GrantList[Ref];
> > +#ifdef GNT_DEBUG
> > + ASSERT (!GrantInUseList[Ref]);
> > + GrantInUseList[Ref] = TRUE;
> > +#endif
> > + /* local_irq_restore(flags); */
> > + return Ref;
> > +}
> > +
> > +STATIC
> > +grant_ref_t
> > +XenGrantTableGrantAccess (
> > + IN domid_t DomainId,
> > + IN UINTN Frame,
> > + IN BOOLEAN ReadOnly
> > + )
> > +{
> > + grant_ref_t Ref;
> > + UINT32 Flags;
> > +
> > + ASSERT (GrantTable != NULL);
> > + Ref = XenGrantTableGetFreeEntry ();
> > + GrantTable[Ref].frame = Frame;
> > + GrantTable[Ref].domid = DomainId;
> > + MemoryFence ();
> > + Flags = GTF_permit_access;
> > + if (ReadOnly) {
> > + Flags |= GTF_readonly;
> > + }
> > + GrantTable[Ref].flags = Flags;
> > +
> > + return Ref;
> > +}
> > +
> > +STATIC
> > +EFI_STATUS
> > +XenGrantTableEndAccess (
> > + grant_ref_t Ref
> > + )
> > +{
> > + UINT16 Flags, OldFlags;
> > +
> > + ASSERT (GrantTable != NULL);
> > + ASSERT (Ref >= NR_RESERVED_ENTRIES && Ref < NR_GRANT_ENTRIES);
> > +
> > + OldFlags = GrantTable[Ref].flags;
> > + do {
> > + if ((Flags = OldFlags) & (GTF_reading | GTF_writing)) {
> > + DEBUG ((EFI_D_WARN, "WARNING: g.e. still in use! (%x)\n", Flags));
> > + return EFI_NOT_READY;
> > + }
> > + OldFlags = InterlockedCompareExchange16 (&GrantTable[Ref].flags,
> > Flags, 0);
> > + } while (OldFlags != Flags);
> > +
> > + XenGrantTablePutFreeEntry (Ref);
> > + return EFI_SUCCESS;
> > +}
> > +
> > +VOID
> > +XenGrantTableInit (
> > + IN XENBUS_DEVICE *Dev,
> > + IN UINT64 MmioAddr
> > + )
> > +{
> > + xen_add_to_physmap_t Args;
> > + INTN Index;
> > + INTN ReturnCode;
> > +
> > +#ifdef GNT_DEBUG
> > + SetMem(GrantInUseList, sizeof (GrantInUseList), 1);
> > +#endif
> > + for (Index = NR_RESERVED_ENTRIES; Index < NR_GRANT_ENTRIES; Index++) {
> > + XenGrantTablePutFreeEntry (Index);
> > + }
> > +
> > + GrantTable = (VOID*)(UINTN) MmioAddr;
> > + for (Index = 0; Index < NR_GRANT_FRAMES; Index++) {
> > + Args.domid = DOMID_SELF;
> > + Args.idx = Index;
> > + Args.space = XENMAPSPACE_grant_table;
> > + Args.gpfn = (((xen_pfn_t) GrantTable) >> EFI_PAGE_SHIFT) + Index;
> > + ReturnCode = XenHypercallMemoryOp (Dev, XENMEM_add_to_physmap, &Args);
> > + if (ReturnCode != 0) {
> > + DEBUG ((EFI_D_ERROR, "Xen GrantTable, add_to_physmap hypercall
> > error: %d\n", ReturnCode));
> > + }
> > + }
> > +}
> > +
> > +VOID
> > +XenGrantTableDeinit (
> > + XENBUS_DEVICE *Dev
> > + )
> > +{
> > + INTN ReturnCode, Index;
> > + xen_remove_from_physmap_t Args;
> > +
> > + if (GrantTable == NULL) {
> > + return;
> > + }
> > +
> > + for (Index = NR_GRANT_FRAMES - 1; Index >= 0; Index--) {
> > + Args.domid = DOMID_SELF;
> > + Args.gpfn = (((xen_pfn_t) GrantTable) >> EFI_PAGE_SHIFT) + Index;
> > + DEBUG ((EFI_D_INFO, "%a %d, removing %X\n", __func__, __LINE__,
> > Args.gpfn));
> > + ReturnCode = XenHypercallMemoryOp (Dev, XENMEM_remove_from_physmap,
> > &Args);
> > + if (ReturnCode != 0) {
> > + DEBUG ((EFI_D_ERROR, "Xen GrantTable, remove_from_physmap hypercall
> > error: %d\n", ReturnCode));
> > + }
> > + }
> > + // XXX remove it from the physmap?
>
> Didn't you just do that?
Yes, a comment to remove...
> > + GrantTable = NULL;
> > +}
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +XenbusGrantAccess (
> > + IN XENBUS_PROTOCOL *This,
> > + IN domid_t DomainId,
> > + IN UINTN Frame, // MFN
> > + IN BOOLEAN ReadOnly,
> > + OUT grant_ref_t *RefPtr
> > + )
> > +{
> > + *RefPtr = XenGrantTableGrantAccess (DomainId, Frame, ReadOnly);
> > + return EFI_SUCCESS;
> > +}
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +XenbusGrantEndAccess (
> > + IN XENBUS_PROTOCOL *This,
> > + IN grant_ref_t Ref
> > + )
> > +{
> > + return XenGrantTableEndAccess (Ref);
> > +}
> > diff --git a/OvmfPkg/XenbusDxe/GrantTable.h b/OvmfPkg/XenbusDxe/GrantTable.h
> > new file mode 100644
> > index 0000000..05ab4be
> > --- /dev/null
> > +++ b/OvmfPkg/XenbusDxe/GrantTable.h
> > @@ -0,0 +1,34 @@
> > +#ifndef __GNTTAB_H__
> > +#define __GNTTAB_H__
> > +
> > +#include <IndustryStandard/Xen/grant_table.h>
> > +
> > +VOID
> > +XenGrantTableInit (
> > + IN XENBUS_DEVICE *Dev,
> > + IN UINT64 MmioAddr
> > + );
> > +
> > +VOID
> > +XenGrantTableDeinit (
> > + IN XENBUS_DEVICE *Dev
> > + );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +XenbusGrantAccess (
> > + IN XENBUS_PROTOCOL *This,
> > + IN domid_t DomainId,
> > + IN UINTN Frame, // MFN
> > + IN BOOLEAN ReadOnly,
> > + OUT grant_ref_t *RefPtr
> > + );
> > +
> > +EFI_STATUS
> > +EFIAPI
> > +XenbusGrantEndAccess (
> > + IN XENBUS_PROTOCOL *This,
> > + IN grant_ref_t Ref
> > + );
> > +
> > +#endif /* !__GNTTAB_H__ */
> > diff --git a/OvmfPkg/XenbusDxe/XenbusDxe.c b/OvmfPkg/XenbusDxe/XenbusDxe.c
> > index ba5e8f4..b19055d 100644
> > --- a/OvmfPkg/XenbusDxe/XenbusDxe.c
> > +++ b/OvmfPkg/XenbusDxe/XenbusDxe.c
> > @@ -17,6 +17,7 @@
> > #include "XenbusDxe.h"
> >
> > #include "XenHypercall.h"
> > +#include "GrantTable.h"
> >
> > ///
> > /// Driver Binding Protocol instance
> > @@ -291,6 +292,8 @@ XenbusDxeDriverBindingStart (
> > EFI_STATUS Status;
> > XENBUS_DEVICE *Dev;
> > EFI_PCI_IO_PROTOCOL *PciIo;
> > + EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *BarDesc;
> > + UINT64 MmioAddr;
> >
> > Status = gBS->OpenProtocol (
> > ControllerHandle,
> > @@ -310,12 +313,23 @@ XenbusDxeDriverBindingStart (
> > Dev->ControllerHandle = ControllerHandle;
> > Dev->PciIo = PciIo;
> >
> > + Status = PciIo->GetBarAttributes (PciIo, PCI_BAR_IDX1, NULL, (VOID**)
> > &BarDesc);
>
> I think we have a spec somewhere for this device. Do you think
> it might make sense to reference it here?
Probably, yes. But I'm not sure yet if it's the right things to do here.
I'm just use the BAR address to map the grant table in.
> > + ASSERT_EFI_ERROR (Status);
> > + ASSERT (BarDesc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM);
>
> I sooo hope the spec mandates that it is always this type of BAR.
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |