Re: backing storage for eMMC boot partitions

2024-11-18 Thread Cédric Le Goater




Yes. That's a "problem" to fix for emmc/sd devices. They should only
be created when defaults_enabled(), just like the flash devices.


This is -nodefaults, right? The man-page for that says:
   -nodefaults
  Don't  create default devices. Normally, QEMU sets the de‐
  fault devices like serial  port,  parallel  port,  virtual
  console,  monitor  device,  VGA adapter, floppy and CD-ROM
  drive and others. The -nodefaults option will disable  all
  those default devices.

That sounds like it would require explicit configuration of the devices
on the SoC (such as UARTs) as well? Having to specify them (perhaps
even with their MMIO addresses) doesn't seem convenient. :)


I don't think the -nodefaults option applies to the platform devices.
These are part of the SoC and can not be replaced/removed on a real
config. However, any device attached to some bus, flash, eMMC, I2C,
PCI, etc. is a good candidate. Some would say that these devices
are soldered on the board. This is not always the case (some are in
sockets) and since HW vendors offer boards with different topologies,
I think  we can leverage QEMU to define custom boards on the command
line.


aspeed seems to only (or additionally?) skip creation of the flash
devices with -nodefaults.


Yes. Flash devices were a good (first) candidate. The goal was to
extend testing to all SPI controllers (some SoCs have 8) and also
to limit the proliferation of boards having only one different
flash model.

There is a cost to it :

  - it complexifies the command line
  - the machine boots in "execute-in-place" mode which is what HW
does, so the model is much more precise, but it is also slower
in emulation.

We thought about doing the same for I2C. Given the amount of I2C
devices on some boards, it didn't seem reasonable and the need
for it was limited. We can still attach on the command line I2C
devices and discover them after boot.

Isn't there a way to configure only default devices as disabled? 


That's -nodefaults or I am misunderstanding. Am I ?


Or to
dynamically attach a user-created SD/eMMC device to an existing SD
controller?


QEMU lacks a way to identify the SD bus the user wants to attach the
device to.


In system/vl.c, I found 'default_sdcard', which is disabled in some
cases. I wasn't able to figure out how to use it or if it would help
for this use-case.


May be Philippe can answer. I am not an sd person.

Thanks,

C.




[PATCH for-9.2 1/4] hw/nvme: fix msix_uninit with exclusive bar

2024-11-18 Thread Klaus Jensen
From: Klaus Jensen 

Commit fa905f65c554 introduced a machine compatibility parameter to
enable an exclusive bar for msix. It failed to account for this when
cleaning up. Make sure that if an exclusive bar is enabled, we use the
proper cleanup routine.

Cc: qemu-sta...@nongnu.org
Fixes: fa905f65c554 ("hw/nvme: add machine compatibility parameter to enable 
msix exclusive bar")
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 
69bce20f6692b48ae162ad948b1a3eef31d69ebd..13898d58278ed2155d45a9120fc07ea60bc64a32
 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8904,7 +8904,12 @@ static void nvme_exit(PCIDevice *pci_dev)
 pcie_sriov_pf_exit(pci_dev);
 }
 
-msix_uninit(pci_dev, &n->bar0, &n->bar0);
+if (n->params.msix_exclusive_bar && !pci_is_vf(pci_dev)) {
+msix_uninit_exclusive_bar(pci_dev);
+} else {
+msix_uninit(pci_dev, &n->bar0, &n->bar0);
+}
+
 memory_region_del_subregion(&n->bar0, &n->iomem);
 }
 

-- 
2.45.2




[PATCH for-9.2 0/4] fixes for hw/nvme

2024-11-18 Thread Klaus Jensen
A set of misc fixes related to SR-IOV compliance and MSI-X.

Signed-off-by: Klaus Jensen 
---
Klaus Jensen (4):
  hw/nvme: fix msix_uninit with exclusive bar
  hw/nvme: fix use/unuse of msix vectors
  hw/nvme: SR-IOV VFs must hardwire pci interrupt pin register to zero
  hw/nvme: take a reference on the subsystem on vf realization

 hw/nvme/ctrl.c | 27 +++
 1 file changed, 23 insertions(+), 4 deletions(-)
---
base-commit: abb1565d3d863cf210f18f70c4a42b0f39b8ccdb
change-id: 2024-nvme-fixes-1038fe44e48a

Best regards,
-- 
Klaus Jensen 




[PATCH for-9.2 2/4] hw/nvme: fix use/unuse of msix vectors

2024-11-18 Thread Klaus Jensen
From: Klaus Jensen 

Only call msix_{un,}use_vector() when interrupts are actually enabled
for a completion queue.

Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 
13898d58278ed2155d45a9120fc07ea60bc64a32..a38f460a78599bc191c17a2a376e865a74744e58
 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -5423,7 +5423,7 @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n)
 event_notifier_set_handler(&cq->notifier, NULL);
 event_notifier_cleanup(&cq->notifier);
 }
-if (msix_enabled(pci)) {
+if (msix_enabled(pci) && cq->irq_enabled) {
 msix_vector_unuse(pci, cq->vector);
 }
 if (cq->cqid) {
@@ -5464,9 +5464,10 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, 
uint64_t dma_addr,
 {
 PCIDevice *pci = PCI_DEVICE(n);
 
-if (msix_enabled(pci)) {
+if (msix_enabled(pci) && irq_enabled) {
 msix_vector_use(pci, vector);
 }
+
 cq->ctrl = n;
 cq->cqid = cqid;
 cq->size = size;

-- 
2.45.2




Re: [PATCH v4 0/5] block: allow commit to unmap zero blocks

2024-11-18 Thread Vladimir Sementsov-Ogievskiy

On 26.10.24 19:30, Vincent Vanlaer wrote:

This patch series adds support for zero blocks in non-active commits.
The first three patches in the series refactor the relevant code, patch
four makes the actual changes, and the last patch adds a test for the
new functionality.

---

Changes since v3:
- minor reformating based on checkpatch.pl
- moved tracepoint in commit_iteration before first possible return on
   error
- renamed the handle_error label in commit_iteration to fail and
   prevented the happy path from passing through this label
- moved test script to the tests/qemu-iotests/tests folder and named it
   commit-zero-blocks

Changes since v2:
- moved main loop of commit_run to a separate function and refactored
   the error handling.
- call blk_co_pwrite_zero even if the size of the zero region does not
   align with the sectors of the base image. This removes the need for
   the CommitMethod enum

Changes since v1:
- split up the implementation in three separate commits
- removed accidentally left over includes from testing

Vincent Vanlaer (5):
   block: get type of block allocation in commit_run
   block: move commit_run loop to separate function
   block: refactor error handling of commit_iteration
   block: allow commit to unmap zero blocks
   block: add test non-active commit with zeroed data

  block/commit.c| 116 +-
  tests/qemu-iotests/tests/commit-zero-blocks   |  96 +++
  .../qemu-iotests/tests/commit-zero-blocks.out |  54 
  3 files changed, 232 insertions(+), 34 deletions(-)
  create mode 100755 tests/qemu-iotests/tests/commit-zero-blocks
  create mode 100644 tests/qemu-iotests/tests/commit-zero-blocks.out



Thanks, applied to my block branch,

with

diff --git a/block/commit.c b/block/commit.c
index 5c24c8b80a..bfd969b13f 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -172,7 +172,8 @@ static int commit_iteration(CommitBlockJob *s, int64_t 
offset,
 *requested_bytes = bytes;

 return 0;
-fail:;
+
+fail:
 BlockErrorAction action = block_job_error_action(&s->common, 
s->on_error,
  error_in_source, 
-ret);
 if (action == BLOCK_ERROR_ACTION_REPORT) {

change to patch 3.


Unfortunately, I've missed soft-freeze on 05.11. Will send PR when 10.0 
development phase opens.

--
Best regards,
Vladimir




Re: [PATCH v2] vpc: Read images exported from Azure correctly

2024-11-18 Thread Eric Blake
On Mon, Nov 18, 2024 at 03:36:46PM +0100, Vitaly Kuznetsov wrote:
> It was found that 'qemu-nbd' is not able to work with some disk images
> exported from Azure. Looking at the 512b footer (which contains VPC
> metadata):
> 
>   63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00  |conectix|
> 0010  ff ff ff ff ff ff ff ff  2e c7 9b 96 77 61 00 00  |wa..|
> 0020  00 07 00 00 57 69 32 6b  00 00 00 01 40 00 00 00  |Wi2k@...|
> 0030  00 00 00 01 40 00 00 00  28 a2 10 3f 00 00 00 02  |@...(..?|
> 0040  ff ff e7 47 8c 54 df 94  bd 35 71 4c 94 5f e5 44  |...G.T...5qL._.D|
> 0050  44 53 92 1a 00 00 00 00  00 00 00 00 00 00 00 00  |DS..|
> 0060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
> 
> we can see that Azure uses a different 'Creator application' --
> 'wa\0\0' (offset 0x1c, likely reads as 'Windows Azure') and QEMU uses this
> field to determine how it can get image size. Apparently, Azure uses 'new'
> method, just like Hyper-V.
> 
> Signed-off-by: Vitaly Kuznetsov 
> ---
> Alternatively, we can probably make 'current_size' the default and only use
> CHS for 'vpc '/'qemu'.
> ---
>  block/vpc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block/vpc.c b/block/vpc.c
> index d95a204612b7..b67798697c15 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -321,6 +321,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
> int flags,
>   *  'qemu'  :  CHS  QEMU (uses disk geometry)
>   *  'qem2'  :  current_size QEMU (uses current_size)
>   *  'win '  :  current_size Hyper-V
> + *  'wa\0\0':  current_size Azure
>   *  'd2v '  :  current_size Disk2vhd
>   *  'tap\0' :  current_size XenServer
>   *  'CTXS'  :  current_size XenConverter
> @@ -330,6 +331,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
> int flags,
>   *  that have CHS geometry of the maximum size.
>   */
>  use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
> +   !!memcmp(footer->creator_app, "wa\0", 4) &&

While this is literally correct (a string literal with 3 characters
spelled out includes the implicit NUL byte; sizeof("wa\0") == 4), it
is a bit odd to see a memcmp() of 4 bytes against a literal containing
only 3 characters, especially when the comments above spelled it out
with four characters.  For the sake of avoiding further confusion, it
might be nice to use memcmp() against explicit 4-byte patterns for all
of the strings (not just the Azure witness).

> !!strncmp(footer->creator_app, "qem2", 4) &&
> !!strncmp(footer->creator_app, "d2v ", 4) &&
> !!strncmp(footer->creator_app, "CTXS", 4) &&

I also don't know if it would be any easier to read by creating a
`static const char table[][4] = { "qemu", "qem2", "wa", ...}` (where
you don't have to write any explicit \0, because the compiler is
guaranteed to NUL-pad short strings into the char[4] table entry) and
then write a loop over each entry in the table, rather than having to
spell out a separate strncmp/memcmp line for each string in the table.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




Re: [PATCH] vpc: Read images exported from Azure correctly

2024-11-18 Thread Vitaly Kuznetsov
Vitaly Kuznetsov  writes:

> It was found that 'qemu-nbd' is not able to work with some disk images
> exported from Azure. Looking at the 512b footer (which contains VPC
> metadata):
>
>   63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00  |conectix|
> 0010  ff ff ff ff ff ff ff ff  2e c7 9b 96 77 61 00 00  |wa..|
> 0020  00 07 00 00 57 69 32 6b  00 00 00 01 40 00 00 00  |Wi2k@...|
> 0030  00 00 00 01 40 00 00 00  28 a2 10 3f 00 00 00 02  |@...(..?|
> 0040  ff ff e7 47 8c 54 df 94  bd 35 71 4c 94 5f e5 44  |...G.T...5qL._.D|
> 0050  44 53 92 1a 00 00 00 00  00 00 00 00 00 00 00 00  |DS..|
> 0060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||
>
> we can see that Azure uses a different 'Creator application' --
> 'wa\0\0' (offset 0x1c, likely reads as 'Windows Azure') and QEMU uses this
> field to determine how it can get image size. Apparently, Azure uses 'new'
> method, just like Hyper-V.
>
> Signed-off-by: Vitaly Kuznetsov 
> ---
> Alternatively, we can probably make 'current_size' the default and only use
> CHS for 'vpc '/'qemu'.
> ---
>  block/vpc.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/block/vpc.c b/block/vpc.c
> index d95a204612b7..da8662402d00 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -321,6 +321,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
> int flags,
>   *  'qemu'  :  CHS  QEMU (uses disk geometry)
>   *  'qem2'  :  current_size QEMU (uses current_size)
>   *  'win '  :  current_size Hyper-V
> + *  'wa\0\0':  current_size Azure
>   *  'd2v '  :  current_size Disk2vhd
>   *  'tap\0' :  current_size XenServer
>   *  'CTXS'  :  current_size XenConverter
> @@ -330,6 +331,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
> int flags,
>   *  that have CHS geometry of the maximum size.
>   */
>  use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
> +   !!strncmp(footer->creator_app, "wa", 4) &&

Stupid me forgot how strncmp() works wrt '\0' bytes, I think we're
better off doing plain memcmp() instead. I'll send v2.

> !!strncmp(footer->creator_app, "qem2", 4) &&
> !!strncmp(footer->creator_app, "d2v ", 4) &&
> !!strncmp(footer->creator_app, "CTXS", 4) &&

-- 
Vitaly




[PATCH v2] vpc: Read images exported from Azure correctly

2024-11-18 Thread Vitaly Kuznetsov
It was found that 'qemu-nbd' is not able to work with some disk images
exported from Azure. Looking at the 512b footer (which contains VPC
metadata):

  63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00  |conectix|
0010  ff ff ff ff ff ff ff ff  2e c7 9b 96 77 61 00 00  |wa..|
0020  00 07 00 00 57 69 32 6b  00 00 00 01 40 00 00 00  |Wi2k@...|
0030  00 00 00 01 40 00 00 00  28 a2 10 3f 00 00 00 02  |@...(..?|
0040  ff ff e7 47 8c 54 df 94  bd 35 71 4c 94 5f e5 44  |...G.T...5qL._.D|
0050  44 53 92 1a 00 00 00 00  00 00 00 00 00 00 00 00  |DS..|
0060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ||

we can see that Azure uses a different 'Creator application' --
'wa\0\0' (offset 0x1c, likely reads as 'Windows Azure') and QEMU uses this
field to determine how it can get image size. Apparently, Azure uses 'new'
method, just like Hyper-V.

Signed-off-by: Vitaly Kuznetsov 
---
Alternatively, we can probably make 'current_size' the default and only use
CHS for 'vpc '/'qemu'.
---
 block/vpc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/vpc.c b/block/vpc.c
index d95a204612b7..b67798697c15 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -321,6 +321,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
  *  'qemu'  :  CHS  QEMU (uses disk geometry)
  *  'qem2'  :  current_size QEMU (uses current_size)
  *  'win '  :  current_size Hyper-V
+ *  'wa\0\0':  current_size Azure
  *  'd2v '  :  current_size Disk2vhd
  *  'tap\0' :  current_size XenServer
  *  'CTXS'  :  current_size XenConverter
@@ -330,6 +331,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, 
int flags,
  *  that have CHS geometry of the maximum size.
  */
 use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
+   !!memcmp(footer->creator_app, "wa\0", 4) &&
!!strncmp(footer->creator_app, "qem2", 4) &&
!!strncmp(footer->creator_app, "d2v ", 4) &&
!!strncmp(footer->creator_app, "CTXS", 4) &&
-- 
2.47.0




Re: [PATCH ssh] ssh: Do not switch session to non-blocking mode

2024-11-18 Thread Jakub Jelen
Hi Kevin,
Sorry for the delay, my gmail filters will need some love to handle this
high-traffic mailing lists so I can catch replies ...

On Thu, Nov 14, 2024 at 6:49 PM Kevin Wolf  wrote:
> [...]
> Hm, after looking some more at the code, I agree that it can't have
> worked, for the simple reason that sftp_read() never returns SSH_AGAIN,
> but turns it into 0. Which QEMU would have reported as an I/O error if
> we're not at EOF.
>
> What I don't understand yet where in the code it would have blocked
> before rather than returning an error. I tried to follow the code path
> and didn't see anything like it, but obviously I'm also not familiar
> with libssh code. I guess it also doesn't really matter as long as we
> know it has always been broken...

I think it is the cycle in the sftp_packet_read(), which was polling
until we got
something to read here:

https://gitlab.com/libssh/libssh-mirror/-/blob/master/src/sftp_common.c?ref_type=heads#L75-L100

But I would have to retrace through the code as I already forgot where I
was looking to.

> The thing that maybe misled me is that sftp_recv_response_msg() calls
> ssh_channel_poll() first to make sure that there is even something to
> read. So I expected it should have been non-blocking at least in some
> cases, but if it had been, we would probably have seen I/O errors all
> the time?

Note, that the sftp_recv_response_msg() function is new in master and
not yet in any released versions. The 0.11 version had processing directly
in the sftp_read() (and in each other function handling some responses).

https://gitlab.com/libssh/libssh-mirror/-/blob/stable-0.11/src/sftp.c?ref_type=heads#L1200

Also the ssh_channel_poll() is called only when the file handle is in
non-blocking
mode (whatever it means). The qemu sets the blocking mode here:

https://github.com/qemu/qemu/blob/master/block/ssh.c#L805C1-L805C44

This means the execution goes directly into sftp_read_and_dispatch(), which
we discussed above as it does the blocking.

> > > So I'm not sure if sftp_aio_*() can be combined with something else into
> > > a working solution, and I also don't know if it's affected by the same
> > > libssh bug.
> >
> > Right now, we do not have a full solution. But having SFTP working
> > completely in nonoblocking mode is one of the things we would like to have
> > in the long term.
> >
> > > Jakub, can you help with that?
> > >
> > > [...]
> > >
> > > As far as I can see, libssh sessions aren't thread safe, so we'll have
> > > to make sure to have only one request going at the same time, but I
> > > assume that calling ssh_read/write() from different threads sequentially
> > > isn't a problem?
> >
> > My understanding is that the thread safety of libssh is limited to not
> > sharing session between threads -- there is no synchronization if two
> > threads would send packets at the same time:
> >
> > https://api.libssh.org/master/
> >
> > If you will make sure you will not call sftp_read()/sftp_write() at
> > the same time from different threads, it might work, but it is
> > untested.
>
> How do you feel about it? Do you think this is something libssh can
> support, or is it something that might accidentally work today, but not
> necessarily next year?

If we will write some test coverage for this, I think we can make sure it
keeps working. There is really nothing that libssh would do in the background
so the stuff should not go wrong. We will just make sure to double-check
all the variables are a part of session and not scattered around as static
variables or thread local ones (I hope we don't have much of these though).

> We have a thread pool readily available that we could use, but then
> requests for the same session would come from different threads - just
> never at the same time. If we need a single long-lived thread per
> session instead, that might be a little more involved because we might
> have to implement all the communication and synchronisation from
> scratch.
>
> (Hmm... Or we abuse the IOThread object to create one internally and
> just move the request coroutine to it around libssh calls. That could be
> easy enough.)

Sorry, I don't have much experience around this to bring any useful insight
here.

So going back to the original issue, is the proposed patch something that
could work for you in the short term before a better solution will be
implemented
or is there something we should change?

Jakub




Re: [PATCH] nbd-server: Silence server warnings on port probes

2024-11-18 Thread Eric Blake
On Mon, Nov 18, 2024 at 11:08:51AM -0600, Eric Blake wrote:
> On Fri, Nov 15, 2024 at 01:55:53PM -0600, Eric Blake wrote:
> > While testing the use of qemu-nbd in a Pod of a Kubernetes cluster, I
> > got LOTS of log messages of the forms:
> > 
> > qemu-nbd: option negotiation failed: Failed to read flags: Unexpected 
> > end-of-file before all data were read
> > qemu-nbd: option negotiation failed: Failed to read flags: Unable to read 
> > from socket: Connection reset by peer
> > 

> In testing this as a potential candidate for -rc1, I'm seeing iotests
> failures in `./check 094 119 -nbd` both pre- and post-patch.
> Bisecting now to see if I can find where those tests started
> regressing (looks like a timing change; a "return" line is swapping
> place with a SHUTDOWN event line in the QMP output).

Huh - I'm seeing those same two tests fail on a vanilla v0.9.0
checkout; therefore, this is probably a long-standing regression, and
certainly not something introduced by this patch.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




Re: backing storage for eMMC boot partitions

2024-11-18 Thread Cédric Le Goater

Hello,

[ ... ]


also boots from the emmc controller. How do I provide the
bus and bus index ? "bus=sdhci-bus.2" doesn't work for me.
There is "sd-bus", but it does not have an index.


Yes. Changes are required in the sdhci model and in the boards using it.


I've not really understood how to assemble more complex setups using
qemu's commandline when the board already creates some of the devices.


Yes. That's a "problem" to fix for emmc/sd devices. They should only
be created when defaults_enabled(), just like the flash devices.


Perhaps Cédric can explain how the different boot options are
configured for aspeed?


See https://qemu.readthedocs.io/en/v9.1.0/system/arm/aspeed.html#boot-options
and the avocado/functional tests for examples.

The eMMC case is missing from the docs. Here is a proposal :

  https://lore.kernel.org/qemu-devel/20241118090648.187720-1-...@redhat.com/

Thanks,

C.




I see three cases:
1. specify the blockdev driver and options in the simple case where the
board already creates the SD or eMMC device
2. specify some custom options for the eMMC
3. create a custom eMMC config on a generic machine via sdhci-pci


Case 1 is probably most common. The user has chosen a board and just
wants to boot a rootfs image and doesn't care about boot partitions or
anything else eMMC-specific.


Some users may want to emulate an eMMC with boot partitions, as that
allows them to emulate their physical boards more closely (case 2).
Note that eMMC boot partitions are usually *not* used for storing a
Linux kernel, but for the bootloader (including things like u-boot, TF-
A, OP-TEE, ...). The ROM-code on many SoCs supports loading directly
from eMMC boot partitions. One of the two boot partitions can be
activated with an atomic eMMC EXT CSD register write, allowing atomic
bootloader updates. I think this case was the motivation for Cédric's
eea55625df83 ("aspeed: Introduce a AspeedSoCClass 'boot_from_emmc'
handler").


These users are likely fine with assembling a backing file consisting
of e.g.
- bootloader image (boot0) @ offset 0MiB
- empty space for bootloader updates (boot1) @ offset 1MiB
- partitioned disk image (rootfs, ...) @ offset 2MiB
to get the same setup as their real hardware.


Case 3 is what I want to use the eMMC emulator for: Test eMMC-specific
functionality in Linux userspace, specifically the boot partition
update backend for RAUC, in a CI setup. To improve performance and
because we don't need to emulate any specific board for CI, we use an
x86 guest (q35). As it has PCIe, the easiest way to add the necessary
eMMC emulation is to use sdhci-pci. That was the motivation behind my
patch "hw/sd/sdcard: Allow user creation of eMMCs" [1].

For that case, having one backing file for boot partitions + main area
is fine as well.


If we wanted more flexibility via separate backing files per eMMC
partitions, it might work similar to NVMe Namespaces [2]. For me, that
seems like a lot of complexity a very niche case like eMMC boot
partitions.


Potential future features such as more eMMC data partitions, RPMB
support or separate backing files could be support in QEMU by new eMCC
device options or even additional devices (following the NVMe
approach), without breaking backwards compatibility.

So it seems to me, that Cédric's approach of enabling boot partitions
in hw/arm/aspeed.c only when configured to boot from them via the "hw-
strap1" property should solve cases 1 and 2 without introducing
backwards compatibility issues. Case 3 has explicit configuration (if a
boot partition is emulated), so shouldn't be a problem either.


Thanks,
Jan

[1] 
https://lore.kernel.org/qemu-devel/20241015135649.4189256-1-...@pengutronix.de/T/
[2] 
https://qemu-project.gitlab.io/qemu/system/devices/nvme.html#additional-namespaces







[PATCH 3/7] docs/devel: add b4 for patch retrieval

2024-11-18 Thread Pierrick Bouvier
Signed-off-by: Pierrick Bouvier 
---
 docs/devel/submitting-a-patch.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/devel/submitting-a-patch.rst 
b/docs/devel/submitting-a-patch.rst
index 953682f20cb..d25c677d996 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -417,6 +417,16 @@ For more details on how QEMU's stable process works, refer 
to the
 
 .. _participating_in_code_review:
 
+Retrieve an existing series
+---
+
+If you want to apply an existing series on top of your tree, you can simply use
+`b4 `__.
+
+::
+
+b4 shazam $msg-id
+
 Participating in Code Review
 
 
-- 
2.39.5




[PATCH 6/7] docs: add a glossary

2024-11-18 Thread Pierrick Bouvier
Signed-off-by: Pierrick Bouvier 
---
 docs/devel/control-flow-integrity.rst |   2 +
 docs/devel/multi-thread-tcg.rst   |   2 +
 docs/glossary/index.rst   | 238 ++
 docs/index.rst|   1 +
 docs/system/arm/virt.rst  |   2 +
 docs/system/images.rst|   2 +
 docs/tools/qemu-nbd.rst   |   2 +
 7 files changed, 249 insertions(+)
 create mode 100644 docs/glossary/index.rst

diff --git a/docs/devel/control-flow-integrity.rst 
b/docs/devel/control-flow-integrity.rst
index e6b73a4fe1a..3d5702fa4cc 100644
--- a/docs/devel/control-flow-integrity.rst
+++ b/docs/devel/control-flow-integrity.rst
@@ -1,3 +1,5 @@
+.. _cfi:
+
 
 Control-Flow Integrity (CFI)
 
diff --git a/docs/devel/multi-thread-tcg.rst b/docs/devel/multi-thread-tcg.rst
index d706c27ea74..7fd0a07633d 100644
--- a/docs/devel/multi-thread-tcg.rst
+++ b/docs/devel/multi-thread-tcg.rst
@@ -4,6 +4,8 @@
   This work is licensed under the terms of the GNU GPL, version 2 or
   later. See the COPYING file in the top-level directory.
 
+.. _mttcg:
+
 ==
 Multi-threaded TCG
 ==
diff --git a/docs/glossary/index.rst b/docs/glossary/index.rst
new file mode 100644
index 000..a2d4f3eae16
--- /dev/null
+++ b/docs/glossary/index.rst
@@ -0,0 +1,238 @@
+.. _Glossary:
+
+
+Glossary
+
+
+This section of the manual presents *simply* acronyms and terms QEMU developers
+use.
+
+Accelerator
+---
+
+A specific API used to accelerate execution of guest instructions. It can be
+hardware-based, through a virtualization API provided by the host OS (kvm, hvf,
+whpx, ...) or software-based (tcg). See this description of `supported
+accelerators`.
+
+Board
+-
+
+QEMU system defines board models for various architectures. It's a description
+of a SoC (system-on-chip) with various devices pre-configured, and can be
+selected with the option ``-machine`` of qemu-system.
+For virtual machines, you'll use ``virt`` board model, designed for this use
+case. As an example, for Arm architecture, you can find the `model code
+`_ and
+associated `documentation `.
+
+Block
+-
+
+Block drivers are the available `disk formats ` available, and
+block devices `(see Block device section on options page)`
+are using them to implement disks for a virtual machine.
+
+CFI
+---
+
+Control Flow Integrity is a hardening technique used to prevent exploits
+targeting QEMU by detecting unexpected branches during execution. QEMU 
`actively
+supports` being compiled with CFI enabled.
+
+Device
+--
+
+QEMU is able to emulate a CPU, and all the hardware interacting with it,
+including many devices. When QEMU runs a virtual machine using a hardware-based
+accelerator, it is responsible for emulating, using software, all devices.
+
+EDK2
+
+
+EDK2, as known as `TianoCore `_, is an open source
+implementation of UEFI standard. It's ran by QEMU to support UEFI for virtual
+machines.
+
+gdbstub
+---
+
+QEMU implements a `gdb server `, allowing gdb to attach to it and
+debug a running virtual machine, or a program in user-mode. This allows to 
debug
+a given architecture without having access to hardware.
+
+glib2
+-
+
+`GLib2 `_ is one of the most important library we
+are using through the codebase. It provides many data structures, macros, 
string
+and thread utilities and portable functions across different OS. It's required
+to build QEMU.
+
+Guest agent
+---
+
+`QEMU Guest agent ` is a daemon intended to be executed by guest
+virtual machines and providing various services to help QEMU to interact with
+it.
+
+Guest/Host
+--
+
+Guest is the architecture of the virtual machine, which is emulated.
+Host is the architecture on which QEMU is running on, which is native.
+
+Hypervisor
+--
+
+The formal definition of an hypervisor is a program than can be used to manage 
a
+virtual machine. QEMU itself is an hypervisor.
+
+In the context of QEMU, an hypervisor is an API, provided by the Host OS,
+allowing to execute virtual machines. Linux implementation is KVM (and supports
+Xen as well). For MacOS, it's HVF. Windows defines WHPX. And NetBSD provides
+NVMM.
+
+Migration
+-
+
+QEMU can save and restore the execution of a virtual machine, including across
+different machines. This is provided by the `Migration framework`.
+
+NBD
+---
+
+`QEMU Network Block Device server ` is a tool that can be used to
+mount and access QEMU images, providing functionality similar to a loop device.
+
+Mailing List
+
+
+This is `where `_ all the
+development happens! Changes are posted as series, that all developers can
+review and share feedback for.
+
+For reporting issues, our `GitLab
+

[PATCH 5/7] docs: add a codebase section

2024-11-18 Thread Pierrick Bouvier
Present the various parts of QEMU and organization of codebase.

Signed-off-by: Pierrick Bouvier 
---
 docs/about/emulation.rst   |   2 +
 docs/codebase/index.rst| 211 +
 docs/devel/decodetree.rst  |   2 +
 docs/devel/ebpf_rss.rst|   2 +
 docs/devel/index-internals.rst |   2 +
 docs/devel/migration/main.rst  |   2 +
 docs/devel/qapi-code-gen.rst   |   1 +
 docs/devel/testing/main.rst|   9 +-
 docs/devel/testing/qtest.rst   |   2 +
 docs/index.rst |   3 +
 docs/interop/qemu-ga.rst   |   2 +
 docs/system/qemu-block-drivers.rst.inc |   2 +
 docs/tools/qemu-storage-daemon.rst |   2 +
 docs/user/main.rst |   6 +
 14 files changed, 247 insertions(+), 1 deletion(-)
 create mode 100644 docs/codebase/index.rst

diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst
index 3028d5fff7a..3bc3579434f 100644
--- a/docs/about/emulation.rst
+++ b/docs/about/emulation.rst
@@ -176,6 +176,8 @@ for that architecture.
 - System
 - Tensilica ISS SIMCALL
 
+.. _tcg-plugins:
+
 TCG Plugins
 ---
 
diff --git a/docs/codebase/index.rst b/docs/codebase/index.rst
new file mode 100644
index 000..353830ef175
--- /dev/null
+++ b/docs/codebase/index.rst
@@ -0,0 +1,211 @@
+
+Codebase
+
+
+This section present the various parts of QEMU and how codebase is organized.
+
+Beyond giving succint descriptions, the goal is to offer links to various
+parts of the documentation/codebase.
+
+Subsystems
+--
+
+An exhaustive list of subsystems and files associated can be found in
+`MAINTAINERS `_
+file.
+
+Some of the main QEMU subsystems are:
+
+- `Accelerators`
+- Block devices and `disk images` support
+- `CI` and `Tests`
+- `Devices` & Board models
+- `Documentation `
+- `GDB support`
+- `Migration`
+- `Monitor`
+- `QOM (QEMU Object Model)`
+- `System mode`
+- `TCG (Tiny Code Generator)`
+- `User mode` (`Linux` & `BSD`)
+- User Interfaces
+
+More documentation on QEMU subsystems can be found on :ref:`internal-subsystem`
+page.
+
+The Grand tour
+--
+
+We present briefly here what every folder of the codebase contains. Hop on!
+
+* `accel `_:
+  Infrastructure and architecture agnostic code related to the various
+  `accelerators ` supported by QEMU
+  (TCG, KVM, hvf, whpx, xen, nvmm).
+  Contains interfaces for operations that will be implemented per
+  `target `_.
+* `audio `_:
+  Audio (host) support.
+* `authz `_:
+  `QEMU Authorization framework`.
+* `backends `_:
+  Various backends used for device emulation.
+* `block `_:
+  Block devices and `image formats` implementation.
+* `bsd-user `_:
+  `BSD User mode`.
+* build: Where the code built goes!
+* `chardev `_:
+  Various backends used by char devices.
+* `common-user 
`_:
+  User-mode assembly code for dealing with signals occuring during syscalls.
+* `configs `_:
+  Makefiles defining configurations to build QEMU.
+* `contrib `_:
+  Community contributed devices/plugins/tools.
+* `crypto `_:
+  Cryptographic algorithms used in QEMU.
+* `disas `_:
+  Disassembly functions used by QEMU target code.
+* `docs `_:
+  QEMU Documentation.
+* `dump `_:
+  Code to dump memory of a running VM.
+* `ebpf `_:
+  eBPF program support in QEMU. `virtio-net RSS` uses it.
+* `fpu `_:
+  Floating-point software emulation.
+* `fsdev `_:
+  `VirtFS `_ support.
+* `gdbstub `_:
+  `GDB ` support.
+* `gdb-xml `_:
+  Set of XML files describing architectures and used by `gdbstub `.
+* `host `_:
+  Various architecture specific header files (crypto, atom

[PATCH 4/7] docs/devel: add information on how to setup build environments

2024-11-18 Thread Pierrick Bouvier
MacOS and Linux are straightforward, but Windows needs a bit more
details.

Signed-off-by: Pierrick Bouvier 
---
 docs/about/build-platforms.rst   |   4 +-
 docs/devel/build-environment.rst | 114 +++
 docs/devel/index-build.rst   |   1 +
 3 files changed, 118 insertions(+), 1 deletion(-)
 create mode 100644 docs/devel/build-environment.rst

diff --git a/docs/about/build-platforms.rst b/docs/about/build-platforms.rst
index 6102f00aec0..c1ea53db834 100644
--- a/docs/about/build-platforms.rst
+++ b/docs/about/build-platforms.rst
@@ -29,6 +29,9 @@ The `Repology`_ site is a useful resource to identify
 currently shipped versions of software in various operating systems,
 though it does not cover all distros listed below.
 
+You can find how to install build dependencies for different systems on the
+:ref:`setup-build-env` page.
+
 Supported host architectures
 
 
@@ -130,7 +133,6 @@ Optional build dependencies
   cross compilation using ``docker`` or ``podman``, or to use pre-built
   binaries distributed with QEMU.
 
-
 Windows
 ---
 
diff --git a/docs/devel/build-environment.rst b/docs/devel/build-environment.rst
new file mode 100644
index 000..d9a66f5fcc6
--- /dev/null
+++ b/docs/devel/build-environment.rst
@@ -0,0 +1,114 @@
+
+.. _setup-build-env:
+
+Setup build environment
+===
+
+QEMU uses a lot of dependencies on the host system. glib2 is used everywhere in
+the code base, and most of the other dependencies are optional.
+
+We present here simple instructions to enable native builds on most popular
+systems.
+
+You can find additional instructions on `QEMU wiki `_:
+
+- `Linux `_
+- `MacOS `_
+- `Windows `_
+- `BSD `_
+
+Linux
+-
+
+Fedora
+++
+
+::
+
+sudo dnf update && sudo dnf builddep qemu
+
+Debian/Ubuntu
++
+
+You first need to enable `Sources List `_.
+Then, use apt to install dependencies:
+
+::
+
+sudo apt update && sudo apt build-dep qemu
+
+MacOS
+-
+
+You first need to install `Homebrew `_. Then, use it to
+install dependencies:
+
+::
+
+brew update && brew install $(brew deps --include-build qemu)
+
+Windows
+---
+
+You first need to install `MSYS2 `_.
+MSYS2 offers `different environments 
`_.
+x86_64 environments are based on GCC, while aarch64 is based on Clang.
+
+We recommend to use MINGW64 for windows-x86_64 and CLANGARM64 for 
windows-aarch64
+(only available on windows-aarch64 hosts).
+
+Then, you can open a windows shell, and enter msys2 env using:
+
+::
+
+c:/msys64/msys2_shell.cmd -defterm -here -no-start -mingw64
+# Replace -ucrt64 by -clangarm64 or -ucrt64 for other environments.
+
+MSYS2 package manager does not offer a built-in way to install build
+dependencies. You can start with this list of packages using pacman:
+
+Note: Dependencies need to be installed again if you use a different MSYS2
+environment.
+
+::
+
+# update MSYS2 itself, you need to reopen your shell at the end.
+pacman -Syu
+pacman -S \
+base-devel binutils bison diffutils flex git grep make sed \
+${MINGW_PACKAGE_PREFIX}-toolchain \
+${MINGW_PACKAGE_PREFIX}-glib2 \
+${MINGW_PACKAGE_PREFIX}-gtk3 \
+${MINGW_PACKAGE_PREFIX}-libnfs \
+${MINGW_PACKAGE_PREFIX}-libssh \
+${MINGW_PACKAGE_PREFIX}-ninja \
+${MINGW_PACKAGE_PREFIX}-pixman \
+${MINGW_PACKAGE_PREFIX}-pkgconf \
+${MINGW_PACKAGE_PREFIX}-python \
+${MINGW_PACKAGE_PREFIX}-SDL2 \
+${MINGW_PACKAGE_PREFIX}-zstd
+
+If you want to install all dependencies, it's possible to use recipe used to
+build QEMU in MSYS2 itself.
+
+::
+
+pacman -S wget
+wget 
https://raw.githubusercontent.com/msys2/MINGW-packages/refs/heads/master/mingw-w64-qemu/PKGBUILD
+# Some packages may be missing for your environment, installation will 
still
+# be done though.
+makepkg -s PKGBUILD || true
+
+Build on windows-aarch64
+
+
+When trying to cross compile meson for x86_64 using UCRT64 or MINGW64 env,
+configure will run into an error because the cpu detected is not correct.
+
+Meson detects x86_64 processes emulated, so you need to manually set the cpu,
+and force a cross compilation (with empty prefix).
+
+::
+
+./configure --cpu=x86_64 --cross-prefix=
+
diff --git a/docs/devel/index-build.rst b/docs/devel/index-build.rst
index 0023953be36..0745c81a264 100644
--- a/docs/devel/index-build.rst
+++ b/docs/devel/index-build.rst
@@ -8,6 +8,7 @@ some of the basics if you are adding new files and targets to 
the build.
:maxdepth: 3
 
build-system
+   build-environment
kconfig
docs
qapi-code-gen
-- 
2.39.5




[PATCH 7/7] docs: add a how to section

2024-11-18 Thread Pierrick Bouvier
Signed-off-by: Pierrick Bouvier 
---
 docs/devel/build-system.rst |   2 +
 docs/how-to/index.rst   | 146 
 docs/index.rst  |   1 +
 3 files changed, 149 insertions(+)
 create mode 100644 docs/how-to/index.rst

diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
index d42045a2325..db444787e37 100644
--- a/docs/devel/build-system.rst
+++ b/docs/devel/build-system.rst
@@ -1,3 +1,5 @@
+.. _build:
+
 ==
 The QEMU build system architecture
 ==
diff --git a/docs/how-to/index.rst b/docs/how-to/index.rst
new file mode 100644
index 000..3a9d4d777df
--- /dev/null
+++ b/docs/how-to/index.rst
@@ -0,0 +1,146 @@
+.. _how-to:
+
+--
+How to
+--
+
+This section of the manual will give you some commands to do various tasks with
+QEMU. It does not intend to be complete, but to be simple.
+
+Build
+-
+
+First you need setup your `build environment `.
+
+Then, you can build QEMU using:
+
+::
+
+git clone https://gitlab.com/qemu-project/qemu
+cd qemu
+./configure
+ninja -C build
+# all binaries are in ./build
+
+By default, QEMU build is optimized. You may want to switch to debug builds
+instead (non optimized, and with more runtime checks enabled):
+
+::
+
+./configure --enable-debug
+
+It's recommended to use sanitizers to catch issues when developing your change.
+
+::
+
+./configure --enable-asan --enable-ubsan
+# Of course, you can combine debug and sanitizers if needed
+
+You can find more information on `build page `.
+
+Test
+
+
+QEMU has a lot of tests, mainly in 4 categories:
+
+::
+
+# run tests related to TCG. They are based on Makefiles.
+make check-tcg
+# run system tests, running a full VM, with avocado framework
+make check-avocado
+# run functional tests, running a full VM, integrated with Meson
+make check-functional
+# run all other tests, integrated with Meson
+make check
+
+You can find more information on `testing page`.
+
+Use QEMU
+
+
+To create a 20 gigabytes disk image usable with qemu-system:
+
+::
+
+qemu-img create system.img 20g
+
+To run an x86_64 system emulated, with 4 cpus, 8G of memory and an install iso:
+
+::
+
+qemu-system-x86_64 -smp 4 -m 8G system.img -cdrom install.iso
+
+To boot directly a Linux Kernel:
+
+::
+
+qemu-system-x86_64 -kernel bzImage -hda system.img -append "root=/dev/hda"
+
+To boot an aarch64 system emulated, you need to specify a UEFI and associated
+pflash. Once started, you can switch to Serial output by clicking on View ->
+Serial0.
+
+::
+
+# UEFI can be obtained from debian package qemu-efi-aarch64.
+# First, we need to copy a file to save UEFI variables:
+# cp /usr/share/AAVMF/AAVMF_VARS.fd .
+qemu-system-aarch64 \
+-m 8G \
+-smp 4 \
+-M virt \
+-cpu max \
+-device virtio-blk-pci,drive=root \
+-drive if=none,id=root,file=system.img \
+-drive if=pflash,readonly=on,file=/usr/share/AAVMF/AAVMF_CODE.fd \
+-drive if=pflash,file=AAVMF_VARS.fd \
+-cdrom install.iso
+
+To run git using QEMU user-mode:
+
+::
+
+qemu-x86_64 /usr/bin/git --version
+
+Contribute
+--
+
+We recommend using `git-publish `_ for
+contributing. You need to configure `git send-email
+`_ first.
+
+::
+
+git checkout -b my_feature
+... # edit, build, test
+# When ready to send the series...
+
+# Add upstream QEMU repo as a remote.
+git remote add upstream https://gitlab.com/qemu-project/qemu
+# Fetch all new content.
+git fetch -a upstream
+
+# Rebase your branch on top of upstream master, and include a signoff.
+git rebase -i upstream/master --signoff
+# Check your patches are correct.
+./scripts/checkpatch.pl $(git merge-base upstream/master HEAD)..HEAD
+
+# Send your series, you'll be given a chance to edit cover letter for it.
+git-publish
+
+# After review, and other changes, you can send a v2 simply by using:
+git-publish
+
+If you need to apply locally an existing series, you can use `b4
+`_ (installable via pip) to retrieve it:
+
+::
+
+b4 shazam 
+# message id is an identifier present in email sent.
+# when using patchwork, it is the last part of a series url (2024...):
+# https://patchew.org/QEMU/20241118021820.4928-1-j...@jms.id.au/
+
+More complete information is available on our `Submit a patch page
+`.
diff --git a/docs/index.rst b/docs/index.rst
index 2cad84cd77c..e275a9223dd 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -20,5 +20,6 @@ Welcome to QEMU's documentation!
interop/index
specs/index
devel/index
+   how-to/index
codebase/index
glossary/index
-- 
2.39.5




[PATCH 2/7] docs/devel: add git-publish for patch submitting

2024-11-18 Thread Pierrick Bouvier
Signed-off-by: Pierrick Bouvier 
---
 docs/devel/submitting-a-patch.rst | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/devel/submitting-a-patch.rst 
b/docs/devel/submitting-a-patch.rst
index 349c32ee3a9..953682f20cb 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -237,6 +237,20 @@ attachments can be used as a last resort on a first-time 
submission.
 
 .. _if_you_cannot_send_patch_emails:
 
+Use git-publish
+~~~
+
+If you already configured git send-email, you can simply use `git-publish
+`__ to send series.
+
+::
+
+$ git checkout master -b my-feature
+$ # work on new commits, add your 'Signed-off-by' lines to each
+$ git publish
+$ ... more work, rebase on master, ...
+$ git publish # will send a v2
+
 If you cannot send patch emails
 ~~~
 
-- 
2.39.5




[PATCH 0/7] Enhance documentation for new developers

2024-11-18 Thread Pierrick Bouvier
This series extends our documentation with new pages to help developers
onboarding on QEMU. It focuses on providing a big picture of QEMU (to a
modest extend).

As such, it was written to be simple, short, easy to understand, and pointing to
more details. It provides another way to dive into details instead of simply
hitting the "search" box.

The first patches enhance the existing developer section. They provide
information about b4 and git-publish, two important tools that I learnt from my
coworkers, and were not presented anywhere, and were really missing IMHO.

Then, we introduce a new Codebase page, presenting (succintly) the various parts
of QEMU, and what every folder of the codebase contains.
We then add a glossary with the most recurrent acronyms we hear in our daily
conversations on the mailing list.
Finally, we add an "How-to" page which present how to build and test qemu, and
how to contribute a patch. It's definitely a repetition of existing information,
but the goal was to have a self contained page with all the commands I run
daily personally, and that someone would be interested to have.

When reviewing, please keep in mind this is targeting someone who discovers
QEMU, and not someone who contributed to the project for several years. What is
obvious for you will not be obvious for a random young developer.

That said, please free to point if something is "false", or "really incomplete".
It can be hard to summarize in one or two sentences complex parts, but that's
the intent here.

Your feedback on content or organization is very welcome!

Thanks,
Pierrick

Pierrick Bouvier (7):
  docs/devel: remove dead video link for sourcehut submit process
  docs/devel: add git-publish for patch submitting
  docs/devel: add b4 for patch retrieval
  docs/devel: add information on how to setup build environments
  docs: add a codebase section
  docs: add a glossary
  docs: add a how to section

 docs/about/build-platforms.rst |   4 +-
 docs/about/emulation.rst   |   2 +
 docs/codebase/index.rst| 211 ++
 docs/devel/build-environment.rst   | 114 
 docs/devel/build-system.rst|   2 +
 docs/devel/control-flow-integrity.rst  |   2 +
 docs/devel/decodetree.rst  |   2 +
 docs/devel/ebpf_rss.rst|   2 +
 docs/devel/index-build.rst |   1 +
 docs/devel/index-internals.rst |   2 +
 docs/devel/migration/main.rst  |   2 +
 docs/devel/multi-thread-tcg.rst|   2 +
 docs/devel/qapi-code-gen.rst   |   1 +
 docs/devel/submitting-a-patch.rst  |  29 ++-
 docs/devel/testing/main.rst|   9 +-
 docs/devel/testing/qtest.rst   |   2 +
 docs/glossary/index.rst| 238 +
 docs/how-to/index.rst  | 146 +++
 docs/index.rst |   5 +
 docs/interop/qemu-ga.rst   |   2 +
 docs/system/arm/virt.rst   |   2 +
 docs/system/images.rst |   2 +
 docs/system/qemu-block-drivers.rst.inc |   2 +
 docs/tools/qemu-nbd.rst|   2 +
 docs/tools/qemu-storage-daemon.rst |   2 +
 docs/user/main.rst |   6 +
 26 files changed, 788 insertions(+), 6 deletions(-)
 create mode 100644 docs/codebase/index.rst
 create mode 100644 docs/devel/build-environment.rst
 create mode 100644 docs/glossary/index.rst
 create mode 100644 docs/how-to/index.rst

-- 
2.39.5




Re: [PATCH v2] vpc: Read images exported from Azure correctly

2024-11-18 Thread Vitaly Kuznetsov
Eric Blake  writes:

> On Mon, Nov 18, 2024 at 03:36:46PM +0100, Vitaly Kuznetsov wrote:
>> It was found that 'qemu-nbd' is not able to work with some disk images
>> exported from Azure. Looking at the 512b footer (which contains VPC
>> metadata):
>> 
>>   63 6f 6e 65 63 74 69 78  00 00 00 02 00 01 00 00  
>> |conectix|
>> 0010  ff ff ff ff ff ff ff ff  2e c7 9b 96 77 61 00 00  
>> |wa..|
>> 0020  00 07 00 00 57 69 32 6b  00 00 00 01 40 00 00 00  
>> |Wi2k@...|
>> 0030  00 00 00 01 40 00 00 00  28 a2 10 3f 00 00 00 02  
>> |@...(..?|
>> 0040  ff ff e7 47 8c 54 df 94  bd 35 71 4c 94 5f e5 44  
>> |...G.T...5qL._.D|
>> 0050  44 53 92 1a 00 00 00 00  00 00 00 00 00 00 00 00  
>> |DS..|
>> 0060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  
>> ||
>> 
>> we can see that Azure uses a different 'Creator application' --
>> 'wa\0\0' (offset 0x1c, likely reads as 'Windows Azure') and QEMU uses this
>> field to determine how it can get image size. Apparently, Azure uses 'new'
>> method, just like Hyper-V.
>> 
>> Signed-off-by: Vitaly Kuznetsov 
>> ---
>> Alternatively, we can probably make 'current_size' the default and only use
>> CHS for 'vpc '/'qemu'.
>> ---
>>  block/vpc.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/block/vpc.c b/block/vpc.c
>> index d95a204612b7..b67798697c15 100644
>> --- a/block/vpc.c
>> +++ b/block/vpc.c
>> @@ -321,6 +321,7 @@ static int vpc_open(BlockDriverState *bs, QDict 
>> *options, int flags,
>>   *  'qemu'  :  CHS  QEMU (uses disk geometry)
>>   *  'qem2'  :  current_size QEMU (uses current_size)
>>   *  'win '  :  current_size Hyper-V
>> + *  'wa\0\0':  current_size Azure
>>   *  'd2v '  :  current_size Disk2vhd
>>   *  'tap\0' :  current_size XenServer
>>   *  'CTXS'  :  current_size XenConverter
>> @@ -330,6 +331,7 @@ static int vpc_open(BlockDriverState *bs, QDict 
>> *options, int flags,
>>   *  that have CHS geometry of the maximum size.
>>   */
>>  use_chs = (!!strncmp(footer->creator_app, "win ", 4) &&
>> +   !!memcmp(footer->creator_app, "wa\0", 4) &&
>
> While this is literally correct (a string literal with 3 characters
> spelled out includes the implicit NUL byte; sizeof("wa\0") == 4), it
> is a bit odd to see a memcmp() of 4 bytes against a literal containing
> only 3 characters, especially when the comments above spelled it out
> with four characters.  For the sake of avoiding further confusion, it
> might be nice to use memcmp() against explicit 4-byte patterns for all
> of the strings (not just the Azure witness).

Yea, it's just that we already have

!!memcmp(footer->creator_app, "tap", 4))

down below so I decided to stick to the style :-)

>
>> !!strncmp(footer->creator_app, "qem2", 4) &&
>> !!strncmp(footer->creator_app, "d2v ", 4) &&
>> !!strncmp(footer->creator_app, "CTXS", 4) &&
>
> I also don't know if it would be any easier to read by creating a
> `static const char table[][4] = { "qemu", "qem2", "wa", ...}` (where
> you don't have to write any explicit \0, because the compiler is
> guaranteed to NUL-pad short strings into the char[4] table entry) and
> then write a loop over each entry in the table, rather than having to
> spell out a separate strncmp/memcmp line for each string in the table.

I like the idea but I'm still trying to understand whether we need to
keep adding new entries there or just flip the default and say that only
'vpc ' and 'qemu' are legacy and deserve CHS.

-- 
Vitaly




Re: [PATCH] nbd-server: Silence server warnings on port probes

2024-11-18 Thread Eric Blake
On Fri, Nov 15, 2024 at 01:55:53PM -0600, Eric Blake wrote:
> While testing the use of qemu-nbd in a Pod of a Kubernetes cluster, I
> got LOTS of log messages of the forms:
> 
> qemu-nbd: option negotiation failed: Failed to read flags: Unexpected 
> end-of-file before all data were read
> qemu-nbd: option negotiation failed: Failed to read flags: Unable to read 
> from socket: Connection reset by peer
> 
> While it is nice to warn about clients that aren't following protocol
> (in case it helps diagnosing bugs in those clients), a mere port probe
> (where the client never write()s any bytes, and where we might even
> hit EPIPE in trying to send our greeting to the client) is NOT
> abnormal, but merely serves to pollute the log.  And Kubernetes
> _really_ likes to do port probes to determine whether a given Pod is
> up and running.
> 
> Easy ways to demonstrate the above port probes:
> $ qemu-nbd -r -f raw path/to/file &
> $ nc localhost 10809  $ bash -c 'exec  $ kill $!
> 
> Silence the noise by not capturing errors until after our first
> successful read() from a client.
> 
> Signed-off-by: Eric Blake 
> ---
>  nbd/server.c | 26 +-
>  1 file changed, 17 insertions(+), 9 deletions(-)

In testing this as a potential candidate for -rc1, I'm seeing iotests
failures in `./check 094 119 -nbd` both pre- and post-patch.
Bisecting now to see if I can find where those tests started
regressing (looks like a timing change; a "return" line is swapping
place with a SHUTDOWN event line in the QMP output).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




[PATCH 1/7] docs/devel: remove dead video link for sourcehut submit process

2024-11-18 Thread Pierrick Bouvier
Signed-off-by: Pierrick Bouvier 
---
 docs/devel/submitting-a-patch.rst | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/docs/devel/submitting-a-patch.rst 
b/docs/devel/submitting-a-patch.rst
index 83e9092b8c0..349c32ee3a9 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -252,10 +252,7 @@ patches to the QEMU mailing list by following these steps:
 #. Send your patches to the QEMU mailing list using the web-based
``git-send-email`` UI at https://git.sr.ht/~USERNAME/qemu/send-email
 
-`This video
-`__
-shows the web-based ``git-send-email`` workflow. Documentation is
-available `here
+Documentation for sourcehut is available `here
 `__.
 
 .. _cc_the_relevant_maintainer:
-- 
2.39.5




[PULL 1/1] nbd-server: Silence server warnings on port probes

2024-11-18 Thread Eric Blake
While testing the use of qemu-nbd in a Pod of a Kubernetes cluster, I
got LOTS of log messages of the forms:

qemu-nbd: option negotiation failed: Failed to read flags: Unexpected 
end-of-file before all data were read
qemu-nbd: option negotiation failed: Failed to read flags: Unable to read from 
socket: Connection reset by peer

While it is nice to warn about clients that aren't following protocol
(in case it helps diagnosing bugs in those clients), a mere port probe
(where the client never write()s any bytes, and where we might even
hit EPIPE in trying to send our greeting to the client) is NOT
abnormal, but merely serves to pollute the log.  And Kubernetes
_really_ likes to do port probes to determine whether a given Pod is
up and running.

Easy ways to demonstrate the above port probes:
$ qemu-nbd -r -f raw path/to/file &
$ nc localhost 10809 
Message-ID: <20241115195638.1132007-2-ebl...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 nbd/server.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index c30e687fc8b..f64e47270c0 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1150,8 +1150,8 @@ nbd_negotiate_meta_queries(NBDClient *client, Error 
**errp)
  * Return:
  * -errno  on error, errp is set
  * 0   on successful negotiation, errp is not set
- * 1   if client sent NBD_OPT_ABORT, i.e. on valid disconnect,
- * errp is not set
+ * 1   if client sent NBD_OPT_ABORT (i.e. on valid disconnect) or never
+ * wrote anything (i.e. port probe); errp is not set
  */
 static coroutine_fn int
 nbd_negotiate_options(NBDClient *client, Error **errp)
@@ -1175,8 +1175,13 @@ nbd_negotiate_options(NBDClient *client, Error **errp)
 ...   Rest of request
 */

-if (nbd_read32(client->ioc, &flags, "flags", errp) < 0) {
-return -EIO;
+/*
+ * Intentionally ignore errors on this first read - we do not want
+ * to be noisy about a mere port probe, but only for clients that
+ * start talking the protocol and then quit abruptly.
+ */
+if (nbd_read32(client->ioc, &flags, "flags", NULL) < 0) {
+return 1;
 }
 client->mode = NBD_MODE_EXPORT_NAME;
 trace_nbd_negotiate_options_flags(flags);
@@ -1383,8 +1388,8 @@ nbd_negotiate_options(NBDClient *client, Error **errp)
  * Return:
  * -errno  on error, errp is set
  * 0   on successful negotiation, errp is not set
- * 1   if client sent NBD_OPT_ABORT, i.e. on valid disconnect,
- * errp is not set
+ * 1   if client sent NBD_OPT_ABORT (i.e. on valid disconnect) or never
+ * wrote anything (i.e. port probe); errp is not set
  */
 static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
 {
@@ -1415,9 +1420,12 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, 
Error **errp)
 stq_be_p(buf + 8, NBD_OPTS_MAGIC);
 stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);

-if (nbd_write(client->ioc, buf, 18, errp) < 0) {
-error_prepend(errp, "write failed: ");
-return -EINVAL;
+/*
+ * Be silent about failure to write our greeting: there is nothing
+ * wrong with a client testing if our port is alive.
+ */
+if (nbd_write(client->ioc, buf, 18, NULL) < 0) {
+return 1;
 }
 ret = nbd_negotiate_options(client, errp);
 if (ret != 0) {
-- 
2.47.0




Re: backing storage for eMMC boot partitions

2024-11-18 Thread Jan Lübbe
Hi,

On Mon, 2024-11-18 at 10:14 +0100, Cédric Le Goater wrote:
> > > also boots from the emmc controller. How do I provide the
> > > bus and bus index ? "bus=sdhci-bus.2" doesn't work for me.
> > > There is "sd-bus", but it does not have an index.
> 
> Yes. Changes are required in the sdhci model and in the boards using
> it.
> 
> > I've not really understood how to assemble more complex setups
> > using qemu's commandline when the board already creates some of the
> > devices.
> 
> Yes. That's a "problem" to fix for emmc/sd devices. They should only
> be created when defaults_enabled(), just like the flash devices.

This is -nodefaults, right? The man-page for that says:
  -nodefaults
 Don't  create default devices. Normally, QEMU sets the de‐
 fault devices like serial  port,  parallel  port,  virtual
 console,  monitor  device,  VGA adapter, floppy and CD-ROM
 drive and others. The -nodefaults option will disable  all
 those default devices.

That sounds like it would require explicit configuration of the devices
on the SoC (such as UARTs) as well? Having to specify them (perhaps
even with their MMIO addresses) doesn't seem convenient. :)

aspeed seems to only (or additionally?) skip creation of the flash
devices with -nodefaults.

Isn't there a way to configure only default devices as disabled? Or to
dynamically attach a user-created SD/eMMC device to an existing SD
controller?

In system/vl.c, I found 'default_sdcard', which is disabled in some
cases. I wasn't able to figure out how to use it or if it would help
for this use-case.

Jan

> > Perhaps Cédric can explain how the different boot options are
> > configured for aspeed?
> 
> See
> https://qemu.readthedocs.io/en/v9.1.0/system/arm/aspeed.html#boot-options
> and the avocado/functional tests for examples.
> 
> The eMMC case is missing from the docs. Here is a proposal :
> https://lore.kernel.org/qemu-devel/20241118090648.187720-1-...@redhat.com/
> 
> Thanks,
> 
> C.



[PATCH for-9.2 3/4] hw/nvme: SR-IOV VFs must hardwire pci interrupt pin register to zero

2024-11-18 Thread Klaus Jensen
From: Klaus Jensen 

The PCI Interrupt Pin Register does not apply to VFs and MUST be
hardwired to zero.

Fixes: 44c2c09488db ("hw/nvme: Add support for SR-IOV")
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 
a38f460a78599bc191c17a2a376e865a74744e58..61c114c66d1565696430589aeb27d7c4a5d2220a
 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -656,6 +656,12 @@ static void nvme_irq_check(NvmeCtrl *n)
 if (msix_enabled(pci)) {
 return;
 }
+
+/* vfs does not implement intx */
+if (pci_is_vf(pci)) {
+return;
+}
+
 if (~intms & n->irq_status) {
 pci_irq_assert(pci);
 } else {
@@ -8544,7 +8550,7 @@ static bool nvme_init_pci(NvmeCtrl *n, PCIDevice 
*pci_dev, Error **errp)
 unsigned nr_vectors;
 int ret;
 
-pci_conf[PCI_INTERRUPT_PIN] = 1;
+pci_conf[PCI_INTERRUPT_PIN] = pci_is_vf(pci_dev) ? 0 : 1;
 pci_config_set_prog_interface(pci_conf, 0x2);
 
 if (n->params.use_intel_id) {

-- 
2.45.2




[PATCH for-9.2 4/4] hw/nvme: take a reference on the subsystem on vf realization

2024-11-18 Thread Klaus Jensen
From: Klaus Jensen 

Make sure we grab a reference on the subsystem when a VF is realized.
Otherwise, the subsytem will be unrealized automatically when the VFs
are unregistered and unreffed.

This fixes a latent bug but was not exposed until commit 08f632848008
("pcie: Release references of virtual functions"). This was then fixed
(or rather, hidden) by commit c613ad25125b ("pcie_sriov: Do not manually
unrealize"), but that was then reverted (due to other issues) in commit
b0fdaee5d1ed, exposing the bug yet again.

Cc: qemu-sta...@nongnu.org
Fixes: 08f632848008 ("pcie: Release references of virtual functions")
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ctrl.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 
61c114c66d1565696430589aeb27d7c4a5d2220a..ec754195669598082dd573ff1237dbbfb9b8aff5
 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8841,6 +8841,13 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
  */
 n->params.serial = g_strdup(pn->params.serial);
 n->subsys = pn->subsys;
+
+/*
+ * Assigning this link (strong link) causes an `object_unref` later in
+ * `object_release_link_property`. Increment the refcount to balance
+ * this out.
+ */
+object_ref(OBJECT(pn->subsys));
 }
 
 if (!nvme_check_params(n, errp)) {

-- 
2.45.2