[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 24/29] Ovmf/Xen: add Xen PV console SerialPortLib driver



Unless I'm wrong, please:

On 01/26/15 20:03, Ard Biesheuvel wrote:
> This implements a SerialPortLib instance that wires up to the
> PV console ring used by domU guests. Also imports the required
> upstream Xen io/console.h header.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> ---
>  OvmfPkg/Include/IndustryStandard/Xen/io/console.h                   |  51 
> ++++++++++++++++++++++++++
>  OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c   | 147 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf |  34 
> +++++++++++++++++
>  3 files changed, 232 insertions(+)
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/Xen/io/console.h 
> b/OvmfPkg/Include/IndustryStandard/Xen/io/console.h
> new file mode 100644
> index 000000000000..f1caa9765bcf
> --- /dev/null
> +++ b/OvmfPkg/Include/IndustryStandard/Xen/io/console.h
> @@ -0,0 +1,51 @@
> +/******************************************************************************
> + * console.h
> + *
> + * Console I/O interface for Xen guest OSes.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, 
> and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (c) 2005, Keir Fraser
> + */
> +
> +#ifndef __XEN_PUBLIC_IO_CONSOLE_H__
> +#define __XEN_PUBLIC_IO_CONSOLE_H__
> +
> +typedef UINT32 XENCONS_RING_IDX;
> +
> +#define MASK_XENCONS_IDX(idx, ring) ((idx) & (sizeof(ring)-1))
> +
> +struct xencons_interface {
> +    char in[1024];
> +    char out[2048];
> +    XENCONS_RING_IDX in_cons, in_prod;
> +    XENCONS_RING_IDX out_cons, out_prod;
> +};
> +
> +#endif /* __XEN_PUBLIC_IO_CONSOLE_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git 
> a/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c 
> b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
> new file mode 100644
> index 000000000000..97344dc4efb0
> --- /dev/null
> +++ b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c
> @@ -0,0 +1,147 @@
> +/** @file
> +  Xen console SerialPortLib instance
> +
> +  Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#include <Base.h>
> +#include <Uefi/UefiBaseType.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/SerialPortLib.h>
> +#include <Library/XenHypercallLib.h>
> +
> +#include <IndustryStandard/Xen/io/console.h>
> +#include <IndustryStandard/Xen/hvm/params.h>
> +#include <IndustryStandard/Xen/event_channel.h>
> +
> +STATIC evtchn_send_t              mXenConsoleEventChain;
> +STATIC struct xencons_interface   *mXenConsoleInterface;

add a big fat warning above these variables with static storage
duration. You didn't restrict the UEFI phases in which this library is
usable, hence I'm thinking you'll use it in PEIMs and probably even in
SEC. Writable globals in SEC & PEI depend on those phases running out of
DRAM, not flash, which makes this library specific to PrePi. I think the
variables above merit a comment about this.

No other comments from me for this patch (the R-b from Stefano should
suffice).

Thanks
Laszlo

> +
> +RETURN_STATUS
> +EFIAPI
> +SerialPortInitialize (
> +  VOID
> +  )
> +{
> +  mXenConsoleEventChain.port = (UINT32)XenHypercallHvmGetParam 
> (HVM_PARAM_CONSOLE_EVTCHN);
> +  mXenConsoleInterface = (struct xencons_interface *)(UINTN)
> +    (XenHypercallHvmGetParam (HVM_PARAM_CONSOLE_PFN) << EFI_PAGE_SHIFT);
> +
> +  //
> +  // No point in ASSERT'ing here as we won't be seeing the output
> +  //
> +  return RETURN_SUCCESS;
> +}
> +
> +/**
> +  Write data to serial device.
> +
> +  @param  Buffer           Point of data buffer which need to be written.
> +  @param  NumberOfBytes    Number of output bytes which are cached in Buffer.
> +
> +  @retval 0                Write data failed.
> +  @retval !0               Actual number of bytes written to serial device.
> +
> +**/
> +UINTN
> +EFIAPI
> +SerialPortWrite (
> +  IN UINT8     *Buffer,
> +  IN UINTN     NumberOfBytes
> +  )
> +{
> +  XENCONS_RING_IDX  Consumer, Producer;
> +  UINTN             Sent;
> +
> +  if (!mXenConsoleInterface) {
> +    return 0;
> +  }
> +
> +  Consumer = mXenConsoleInterface->out_cons;
> +  Producer = mXenConsoleInterface->out_prod;
> +
> +  MemoryFence ();
> +
> +  Sent = 0;
> +  while (Sent < NumberOfBytes && ((Producer - Consumer) < sizeof 
> (mXenConsoleInterface->out)))
> +    mXenConsoleInterface->out[MASK_XENCONS_IDX(Producer++, 
> mXenConsoleInterface->out)] = Buffer[Sent++];
> +
> +  MemoryFence ();
> +
> +  mXenConsoleInterface->out_prod = Producer;
> +
> +  if (Sent > 0) {
> +    XenHypercallEventChannelOp (EVTCHNOP_send, &mXenConsoleEventChain);
> +  }
> +
> +  return Sent;
> +}
> +
> +/**
> +  Read data from serial device and save the data in buffer.
> +
> +  @param  Buffer           Point of data buffer which need to be written.
> +  @param  NumberOfBytes    Size of Buffer[].
> +
> +  @retval 0                Read data failed.
> +  @retval !0               Actual number of bytes read from serial device.
> +
> +**/
> +UINTN
> +EFIAPI
> +SerialPortRead (
> +  OUT UINT8     *Buffer,
> +  IN  UINTN     NumberOfBytes
> +)
> +{
> +  XENCONS_RING_IDX  Consumer, Producer;
> +  UINTN             Received;
> +
> +  if (!mXenConsoleInterface) {
> +    return 0;
> +  }
> +
> +  Consumer = mXenConsoleInterface->in_cons;
> +  Producer = mXenConsoleInterface->in_prod;
> +
> +  MemoryFence ();
> +
> +  Received = 0;
> +  while (Received < NumberOfBytes && Consumer < Producer)
> +     Buffer[Received++] = 
> mXenConsoleInterface->in[MASK_XENCONS_IDX(Consumer++, 
> mXenConsoleInterface->in)];
> +
> +  MemoryFence ();
> +
> +  mXenConsoleInterface->in_cons = Consumer;
> +
> +  XenHypercallEventChannelOp (EVTCHNOP_send, &mXenConsoleEventChain);
> +
> +  return Received;
> +}
> +
> +/**
> +  Check to see if any data is available to be read from the debug device.
> +
> +  @retval TRUE       At least one byte of data is available to be read
> +  @retval FALSE      No data is available to be read
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +SerialPortPoll (
> +  VOID
> +  )
> +{
> +  return mXenConsoleInterface &&
> +    mXenConsoleInterface->in_cons != mXenConsoleInterface->in_prod;
> +}
> diff --git 
> a/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf 
> b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf
> new file mode 100644
> index 000000000000..f7925b3e6bc3
> --- /dev/null
> +++ b/OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf
> @@ -0,0 +1,34 @@
> +#/** @file
> +#
> +#  Component description file for XenConsoleSerialPortLib module
> +#
> +#  Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD 
> License
> +#  which accompanies this distribution.  The full text of the license may be 
> found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = XenConsoleSerialPortLib
> +  FILE_GUID                      = 401406DD-BCAC-4B91-9F4E-72A7FEBE4762
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = SerialPortLib
> +
> +[Sources.common]
> +  XenConsoleSerialPortLib.c
> +
> +[LibraryClasses]
> +  BaseLib
> +  XenHypercallLib
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> 


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.