Re: [Qemu-devel] [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device

[Andy Lutomirski]

On Sat, Oct 3, 2015 at 4:28 PM, Gabriel L. Somlo  wrote:
> From: Gabriel Somlo 
>
> Make fw_cfg entries of type "file" available via sysfs. Entries
> are listed under /sys/firmware/qemu_fw_cfg/by_key, in folders
> named after each entry's selector key. Filename, selector value,
> and size read-only attributes are included for each entry. Also,
> a "raw" attribute allows retrieval of the full binary content of
> each entry.
>
> This patch also provides a documentation file outlining the
> guest-side "hardware" interface exposed by the QEMU fw_cfg device.
>

What's the status of "by_name"?  There's a single (presumably
incorrect) mention of it in a comment in this patch.

I would prefer if the kernel populated by_name itself rather than
deferring that to udev, since I'd like to use this facility in virtme,
and I'd like to use fw_cfg very early on boot before I even start
udev.

--Andy

Re: [Qemu-devel] [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device

[Gabriel L. Somlo]

On Tue, Oct 06, 2015 at 10:54:42AM -0700, Andy Lutomirski wrote:
> On Sat, Oct 3, 2015 at 4:28 PM, Gabriel L. Somlo  wrote:
> > From: Gabriel Somlo 
> >
> > Make fw_cfg entries of type "file" available via sysfs. Entries
> > are listed under /sys/firmware/qemu_fw_cfg/by_key, in folders
> > named after each entry's selector key. Filename, selector value,
> > and size read-only attributes are included for each entry. Also,
> > a "raw" attribute allows retrieval of the full binary content of
> > each entry.
> >
> > This patch also provides a documentation file outlining the
> > guest-side "hardware" interface exposed by the QEMU fw_cfg device.
> >
> 
> What's the status of "by_name"?  There's a single (presumably
> incorrect) mention of it in a comment in this patch.
> 
> I would prefer if the kernel populated by_name itself rather than
> deferring that to udev, since I'd like to use this facility in virtme,
> and I'd like to use fw_cfg very early on boot before I even start
> udev.

"by_name" is added with patch 4/4 of the series, which I kept separate
due to the "To udev or not to udev" conversation from earlier. So far
I haven't yet figured out just HOW I'd set it up in udev, but it works
already if done in the kernel :)

Thanks,
--Gabriel

Re: [Qemu-devel] [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device

[Stefan Hajnoczi]

On Sat, Oct 03, 2015 at 07:28:06PM -0400, Gabriel L. Somlo wrote:
> +/* read chunk of given fw_cfg blob (caller responsible for sanity-check) */
> +static inline void fw_cfg_read_blob(u16 key,
> + void *buf, loff_t pos, size_t count)
> +{
> + mutex_lock(_cfg_dev_lock);
> + iowrite16(fw_cfg_sel_endianness(key), fw_cfg_reg_ctrl);
> + while (pos-- > 0)
> + ioread8(fw_cfg_reg_data);
> + ioread8_rep(fw_cfg_reg_data, buf, count);
> + mutex_unlock(_cfg_dev_lock);
> +}

Have you had a chance to play with Marc Mari's fw_cfg DMA interface
patches?  They should make this operation much faster.

https://www.mail-archive.com/qemu-devel@nongnu.org/msg325541.html

Stefan

Re: [Qemu-devel] [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device

[Laszlo Ersek]

On 10/04/15 01:28, Gabriel L. Somlo wrote:
> From: Gabriel Somlo 
> 
> Make fw_cfg entries of type "file" available via sysfs. Entries
> are listed under /sys/firmware/qemu_fw_cfg/by_key, in folders
> named after each entry's selector key. Filename, selector value,
> and size read-only attributes are included for each entry. Also,
> a "raw" attribute allows retrieval of the full binary content of
> each entry.
> 
> This patch also provides a documentation file outlining the
> guest-side "hardware" interface exposed by the QEMU fw_cfg device.
> 
> Signed-off-by: Gabriel Somlo 
> ---
>  .../ABI/testing/sysfs-firmware-qemu_fw_cfg | 167 
>  drivers/firmware/Kconfig   |  10 +
>  drivers/firmware/Makefile  |   1 +
>  drivers/firmware/qemu_fw_cfg.c | 456 
> +
>  4 files changed, 634 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
>  create mode 100644 drivers/firmware/qemu_fw_cfg.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg 
> b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
> new file mode 100644
> index 000..f1ef44e
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-firmware-qemu_fw_cfg
> @@ -0,0 +1,167 @@
> +What:/sys/firmware/qemu_fw_cfg/
> +Date:August 2015
> +Contact: Gabriel Somlo 
> +Description:
> + Several different architectures supported by QEMU (x86, arm,
> + sun4*, ppc/mac) are provisioned with a firmware configuration
> + (fw_cfg) device, used by the host to provide configuration data
> + to the starting guest. While most of this data is meant for use
> + by the guest firmware, starting with QEMU v2.4, guest VMs may
> + be given arbitrary fw_cfg entries supplied directly on the
> + command line, which therefore may be of interest to userspace.
> +
> + === Guest-side Hardware Interface ===
> +
> + The fw_cfg device is available to guest VMs as a register pair
> + (control and data), accessible as either a IO ports or as MMIO
> + addresses, depending on the architecture.
> +
> + --- Control Register ---
> +
> + Width: 16-bit
> + Access: Write-Only
> + Endianness: LE (if IOport) or BE (if MMIO)
> +
> + A write to the control register selects the index for one of
> + the firmware configuration items (or "blobs") available on the
> + fw_cfg device, which can subsequently be read from the data
> + register.
> +
> + Each time the control register is written, an data offset
> + internal to the fw_cfg device will be set to zero. This data
> + offset impacts which portion of the selected fw_cfg blob is
> + accessed by reading the data register, as explained below.
> +
> + --- Data Register ---
> +
> + Width: 8-bit (if IOport), or 8/16/32/64-bit (if MMIO)
> + Access: Read-Only
> + Endianness: string preserving
> +
> + The data register allows access to an array of bytes which
> + represent the fw_cfg blob last selected by a write to the
> + control register.
> +
> + Immediately following a write to the control register, the data
> + offset will be set to zero. Each successful read access to the
> + data register will increment the data offset by the appropriate
> + access width.
> +
> + Each fw_cfg blob has a maximum associated data length. Once the
> + data offset exceeds this maximum length, any subsequent reads
> + via the data register will return 0x00.
> +
> + An N-byte wide read of the data register will return the next
> + available N bytes of the selected fw_cfg blob, as a substring,
> + in increasing address order, similar to memcpy(), zero-padded
> + if necessary should the maximum data length of the selected
> + item be reached, as described above.
> +
> + --- Per-arch Register Details ---
> +
> + -
> + archaccess base ctrlctrldatamax.
> + modeaddress offset  endian  offset  data
> + (bytes) (bytes)
> + -
> + x86*IOport0x510 0   LE  1   1
> + arm MMIO  0x902 8   BE  0   8
> + sun4u   IOport0x510 0   LE  1   1
> + sun4m   MMIO

Re: [Qemu-devel] [PATCH v3 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device

[kbuild test robot]

Hi Gabriel,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please 
ignore]

reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/firmware/qemu_fw_cfg.c:66:25: sparse: constant 0xd0510 is so big 
it is long
>> drivers/firmware/qemu_fw_cfg.c:95:39: sparse: restricted __be16 degrades to 
>> integer
>> drivers/firmware/qemu_fw_cfg.c:95:58: sparse: restricted __le16 degrades to 
>> integer
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:111:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:95:39: sparse: restricted __be16 degrades to 
>> integer
>> drivers/firmware/qemu_fw_cfg.c:95:58: sparse: restricted __le16 degrades to 
>> integer
>> drivers/firmware/qemu_fw_cfg.c:95:39: sparse: restricted __be16 degrades to 
>> integer
>> drivers/firmware/qemu_fw_cfg.c:95:58: sparse: restricted __le16 degrades to 
>> integer
   drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
   drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
   drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
   drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
   drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
   drivers/firmware/qemu_fw_cfg.c:367:25: sparse: cast to restricted __be32
>> drivers/firmware/qemu_fw_cfg.c:368:27: sparse: cast to restricted __be16
>> drivers/firmware/qemu_fw_cfg.c:368:27: sparse: cast to restricted __be16
>> drivers/firmware/qemu_fw_cfg.c:368:27: sparse: cast to restricted __be16
>> drivers/firmware/qemu_fw_cfg.c:368:27: sparse: cast to restricted __be16
>> drivers/firmware/qemu_fw_cfg.c:95:39: sparse: restricted __be16 degrades to 
>> integer
>> drivers/firmware/qemu_fw_cfg.c:95:58: sparse: restricted __le16 degrades to 
>> integer
>> drivers/firmware/qemu_fw_cfg.c:420:22: sparse: cast to restricted __le32

vim +95 drivers/firmware/qemu_fw_cfg.c

60  .size = 0x0a,
61  .ctrl_offset = 0x08,
62  .data_offset = 0x00,
63  .is_mmio = true,
64  }, {
65  .name = "fw_cfg MMIO on sun4m",
  > 66  .base = 0xd0510,
67  .size = 0x03,
68  .ctrl_offset = 0x00,
69  .data_offset = 0x02,
70  .is_mmio = true,
71  }, {
72  .name = "fw_cfg MMIO on ppc/mac",
73  .base = 0xf510,
74  .size = 0x03,
75  .ctrl_offset = 0x00,
76  .data_offset = 0x02,
77  .is_mmio = true,
78  }, { } /* END */
79  };
80  
81  /* fw_cfg device i/o currently selected option set */
82  static struct fw_cfg_access *fw_cfg_mode;
83  
84  /* fw_cfg device i/o register addresses */
85  static void __iomem *fw_cfg_dev_base;
86  static void __iomem *fw_cfg_reg_ctrl;
87  static void __iomem *fw_cfg_reg_data;
88  
89  /* atomic access to fw_cfg device (potentially slow i/o, so using 
mutex) */
90  static DEFINE_MUTEX(fw_cfg_dev_lock);
91  
92  /* pick appropriate endianness for selector key */
93  static inline u16 fw_cfg_sel_endianness(u16 key)
94  {
  > 95  return fw_cfg_mode->is_mmio ? cpu_to_be16(key) : 
cpu_to_le16(key);
96  }
97  
98  /* type for fw_cfg "directory scan" visitor/callback function */
99  typedef int (*fw_cfg_file_callback)(const struct fw_cfg_file *f);
   100  
   101  /* run a given callback on each fw_cfg directory entry */
   102  static int fw_cfg_scan_dir(fw_cfg_file_callback callback)
   103  {
   104  int ret = 0;
   105  u32 count, i;
   106  struct fw_cfg_file f;
   107  
   108  mutex_lock(_cfg_dev_lock);
   109  iowrite16(fw_cfg_sel_endianness(FW_CFG_FILE_DIR), 
fw_cfg_reg_ctrl);
   110  ioread8_rep(fw_cfg_reg_data, , sizeof(count));
 > 111  for (i = 0; i < be32_to_cpu(count); i++) {
   112  ioread8_rep(fw_cfg_reg_data, , sizeof(f));
   113  ret = callback();
   114  if (ret)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation