[PATCH] qga-win: Fix guest-get-fsinfo multi-disks collection

2023-12-26 Thread peng . ji
From: Peng Ji 

When a volume has more than one disk, all disks cannot be
returned correctly because there is not enough malloced memory
for disk extents, so before executing DeviceIoControl for the
second time, get the correct size of the required memory space
to store all disk extents.

Signed-off-by: Peng Ji 
---
 qga/commands-win32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 697c65507c..a1015757d8 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -935,6 +935,8 @@ static GuestDiskAddressList *build_guest_disk_info(char 
*guid, Error **errp)
 DWORD last_err = GetLastError();
 if (last_err == ERROR_MORE_DATA) {
 /* Try once more with big enough buffer */
+size = sizeof(VOLUME_DISK_EXTENTS) +
+   (sizeof(DISK_EXTENT) * (extents->NumberOfDiskExtents - 1));
 g_free(extents);
 extents = g_malloc0(size);
 if (!DeviceIoControl(
-- 
2.27.0




[PATCH] qga-win: Fix guest-get-fsinfo multi-disks collection

2023-12-26 Thread peng . ji
From: Peng Ji 

When a volume has more than one disk, all disks cannot be
returned correctly because there is not enough malloced memory
for disk extents, so before executing DeviceIoControl for the
second time, get the correct size of the required memory space
to store all disk extents.

Signed-off-by: Peng Ji 
---
 qga/commands-win32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 697c65507c..a1015757d8 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -935,6 +935,8 @@ static GuestDiskAddressList *build_guest_disk_info(char 
*guid, Error **errp)
 DWORD last_err = GetLastError();
 if (last_err == ERROR_MORE_DATA) {
 /* Try once more with big enough buffer */
+size = sizeof(VOLUME_DISK_EXTENTS) +
+   (sizeof(DISK_EXTENT) * (extents->NumberOfDiskExtents - 1));
 g_free(extents);
 extents = g_malloc0(size);
 if (!DeviceIoControl(
-- 
2.27.0




[Bug 1119281] Re: The virtio network device breaks UuidCreateSequential()

2023-12-26 Thread Not Applicable
The issue is still there in 2023.
Well since XP's source code had been leaked. I've gone through the source code 
and may have found the cause.

Nowadays UuidCreateSequential should use MAC address when available.
Here I quoted from the link below:
"For security reasons, UuidCreate was modified so that it no longer uses a 
machine's MAC address to generate UUIDs. UuidCreateSequential was introduced to 
allow creation of UUIDs using the MAC address of a machine's Ethernet card."
https://learn.microsoft.com/en-us/windows/win32/api/rpcdce/nf-rpcdce-uuidcreatesequential

Now let take a look at XP's code:
UuidCreateSequential() in dceuuid.cxx:
https://github.com/tongzx/nt5src/blob/daad8a087a4e75422ec96b7911f1df4669989611/Source/XPSP1/NT/com/rpc/runtime/mtrt/dceuuid.cxx#L122

RPC_STATUS RPC_ENTRY
UuidCreateSequential (
OUT UUID PAPI * Uuid
)
/*++

Routine Description:

This routine will create a new UUID (or GUID) which is unique in
time and space.  We will try to guarantee that the UUID (or GUID)
we generate is unique in time and space.  This means that this
routine may fail if we can not generate one which we can guarantee
is unique in time and space.

Arguments:

Uuid - Returns the generated UUID (or GUID).

Return Value:

RPC_S_OK - The operation completed successfully.

RPC_S_UUID_NO_ADDRESS - We were unable to obtain the ethernet or
token ring address for this machine.

RPC_S_UUID_LOCAL_ONLY - On NT & Chicago if we can't get a
network address.  This is a warning to the user, the
UUID is still valid, it just may not be unique on other machines.

RPC_S_OUT_OF_MEMORY - Returned as needed.
--*/
{
RPC_UUID_GENERATE PAPI * RpcUuid = (RPC_UUID_GENERATE PAPI *) Uuid;
RPC_STATUS Status = RPC_S_OK;
static DWORD LastTickCount = 0;

InitializeIfNecessary();

if (GetTickCount()-LastTickCount > MAX_CACHED_UUID_TIME)
{
UuidCachedValues.AllocatedCount = 0;
LastTickCount = GetTickCount();
}

ULARGE_INTEGER Time;
long Delta;

for(;;)
{
Time.QuadPart = UuidCachedValues.Time.QuadPart;

// Copy the static info into the UUID.  We can't do this later
// because the clock sequence could be updated by another thread.

*(unsigned long *)&RpcUuid->ClockSeqHiAndReserved =
*(unsigned long *)&UuidCachedValues.ClockSeqHiAndReserved;
*(unsigned long *)&RpcUuid->NodeId[2] =
*(unsigned long *)&UuidCachedValues.NodeId[2];

Delta = InterlockedDecrement(&UuidCachedValues.AllocatedCount);

if (Time.QuadPart != UuidCachedValues.Time.QuadPart)
{
// If our captured time doesn't match the cache then another
// thread already took the lock and updated the cache. We'll
// just loop and try again.
continue;
}

if (Delta >= 0)
{
break;
}

//
// Allocate block of Uuids.
//

Status = UuidGetValues( &UuidCachedValues );
if (Status == RPC_S_OK)
{
UuidCacheValid = CACHE_VALID;
}
else
{
UuidCacheValid = CACHE_LOCAL_ONLY;
}

if (Status != RPC_S_OK
&& Status != RPC_S_UUID_LOCAL_ONLY)
{
#ifdef DEBUGRPC
if (Status != RPC_S_OUT_OF_MEMORY)
PrintToDebugger("RPC: UuidGetValues returned or raised: %x\n", 
Status);
#endif
ASSERT( (Status == RPC_S_OUT_OF_MEMORY) );


return Status;
}

// Loop
}


Time.QuadPart -= Delta;

RpcUuid->TimeLow = (unsigned long) Time.LowPart;
RpcUuid->TimeMid = (unsigned short) (Time.HighPart & 0x);
RpcUuid->TimeHiAndVersion = (unsigned short)
(( (unsigned short)(Time.HighPart >> 16)
& RPC_UUID_TIME_HIGH_MASK) | RPC_UUID_VERSION);

ASSERT(   Status == RPC_S_OK
   || Status == RPC_S_UUID_LOCAL_ONLY);

if (UuidCacheValid == CACHE_LOCAL_ONLY)
{
return RPC_S_UUID_LOCAL_ONLY;
}

return(Status);
}

UuidGetValues() in uuidsup.cxx:
https://github.com/tongzx/nt5src/blob/daad8a087a4e75422ec96b7911f1df4669989611/Source/XPSP1/NT/com/rpc/runtime/mtrt/uuidsup.cxx#L43

UuidGetValues(
OUT UUID_CACHED_VALUES_STRUCT __RPC_FAR *Values
)
/*++

Routine Description:

This routine allocates a block of uuids for UuidCreate to handout.

Arguments:

Values - Set to contain everything needed to allocate a block of uuids.
 The following fields will be updated here:

NextTimeLow -   Together with LastTimeLow, this denotes the boundaries
of a block of Uuids. The values between NextTimeLow
and LastTimeLow are used in a sequence of Uuids returned
by UuidCreate().

LastTimeLow -   See NextTimeLow.

ClockSequence - Clock sequence fie

[Bug 1119281] Re: The virtio network device breaks UuidCreateSequential()

2023-12-26 Thread Not Applicable
-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1119281

Title:
  The virtio network device breaks UuidCreateSequential()

Status in QEMU:
  Expired

Bug description:
  UuidCreateSequential() usually creates version 1 UUIDs (1) which means
  they contain the main network card's MAC address. However when using a
  virtio network card and driver the UUIDs contain random data instead
  of the guest's MAC address. Changing the network card to either the
  default rtl8139 one or the e1000 one fixes the issue.

  Here is the software I have tested this with:
   * qemu 1.1.2+dfsg-5 and 1.4.0~rc0+dfsg-1exp (from Debian Testing and 
Experimental respectively)
   * The 0.1-49 and 0.1-52 Windows virtio drivers from 
https://alt.fedoraproject.org/pub/alt/virtio-win/latest/images/bin/
   * Both a 32-bit Windows XP guest and a 64-bit Windows 7 one.

  
  Here is how to test for this issue:
  * Set up a Windows guest with a single network card(2), a virtio one and 
install the corresponding driver.

  * Boot the guest and copy the uuidtest.exe file (see attachement) to
  it

  * On the command line, type 'ipconfig /all'. Give you the correct
  network card's MAC address on a line like the one below:

  Physical Address. . . . . . . . . : 52-54-00-C7-0E-97

  * Run uuidtest.exe. It will show the VM returning a UUID with the
  wrong MAC address, and quite possibly even a multicast MAC address!
  (3). In the example below 'f75292c62787' should have been the MAC
  address. Note that on Windows XP UuidCreateSequential() returns
  RPC_S_UUID_LOCAL_ONLY for virtio cards but that on Windows 7 it
  returns 0.

  UuidCreateSequential() returned 0
  uuid={56e1ffe4-71d8-11e2-b1cc-f75292c62787}
  Got a version 1 UUID
  The UUID does not contain a non-multicast MAC address

  * Reboot and notice uuidtest.exe now reports a different value where
  the MAC address should be.

  * Shut down the VM and switch the network card to rtl8139, install the
  drivers, run uuidtest.exe and notice that the last group of digits
  finally contains the correct MAC address.

  
  (1) https://en.wikipedia.org/wiki/Globally_unique_identifier#Algorithm
  (2) Best do it with a single card to avoid confusion over which is the 
primary one.
  (3) If the first byte of the address is odd then this is a multicast address.
  https://en.wikipedia.org/wiki/MAC_address#Address_details

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1119281/+subscriptions




[Bug 1119281] Re: The virtio network device breaks UuidCreateSequential()

2023-12-26 Thread Not Applicable
The issue is still there in 2023.
Well since XP's source code had been leaked. I've gone through the source code 
and may have found the cause.

Nowadays UuidCreateSequential should use MAC address when available.
Here I quoted from the link below:
"For security reasons, UuidCreate was modified so that it no longer uses a 
machine's MAC address to generate UUIDs. UuidCreateSequential was introduced to 
allow creation of UUIDs using the MAC address of a machine's Ethernet card."
https://learn.microsoft.com/en-us/windows/win32/api/rpcdce/nf-rpcdce-uuidcreatesequential

Now let take a look at XP's code:
uuid.c:
https://github.com/tongzx/nt5src/blob/daad8a087a4e75422ec96b7911f1df4669989611/Source/XPSP1/NT/base/ntos/ex/uuid.c#L545
/*++

Copyright (c) 1994-1997  Microsoft Corporation

Module Name:

uuid.c

Abstract:

This module implements the core time and sequence number allocation
for UUIDs (exposed to user mode), as well as complete UUID
creation (exposed to kernel mode only).

  (e.g. RPC Runtime)(e.g. NTFS)
  |  |
  V  V
NtAllocateUuids ExUuidCreate
  |  |
  V  V
  | ExpUuidGetValues
  |  |
  |  |
  +--> ExpAllocateUuids <+


start.cxx:
https://github.com/tongzx/nt5src/blob/daad8a087a4e75422ec96b7911f1df4669989611/Source/XPSP1/NT/com/ole32/dcomss/wrapper/start.cxx#L282C23-L282C41

NtSetUuidSeed (
IN PCHAR Seed
)
/*++

Routine Description:

This routine is used to set the seed used for UUID generation. The seed
will be set by RPCSS at startup and each time a card is replaced.

Arguments:

Seed - Pointer to a six byte buffer

Return Value:

STATUS_SUCCESS is returned if the service is successfully executed.

STATUS_ACCESS_DENIED If caller doesn't have the permissions to make this 
call.
You need to be logged on as Local System in order to call this API.

STATUS_ACCESS_VIOLATION is returned if the Seed could not be read.

--*/


/*++

Copyright (c) 1995 Microsoft Corporation

Module Name:

Start.c

Abstract:

Process init and service controller interaction for dcomss.exe

Author:

Mario Goertzel[MarioGo]

Revision History:

MarioGo06-14-95Cloned from the old endpoint mapper.
MazharM10-12.98Add pnp stuff
TarunA 12-11-98Removed pnpmngr.h
a-sergiv   25-08-99Defined gC2Security for process-wide use

--*/
DealWithDeviceEvent()
/*++
Function Name: DealWithDeviceEvent

Parameters:

Description:

Returns:

--*/
{
UCHAR MacAddress[8];
NTSTATUS NtStatus;

if (getMacAddress(&MacAddress[0]))
{
NtStatus = NtSetUuidSeed((PCHAR) &MacAddress[0]);
}
else
{
CookupNodeId(&MacAddress[0]);

ASSERT(MacAddress[0] & 0x80);

NtStatus = NtSetUuidSeed((PCHAR) &MacAddress[0]);
}

if (!NT_SUCCESS(NtStatus))
{
#if DBG
DbgPrint("NtSetUuidSeed failed\n", NtStatus);
#endif
}

#if !defined(SPX_IPX_OFF)
UpdateSap( SAP_CTRL_UPDATE_ADDRESS );
#endif
}


getMacAddress (
PUCHAR pMacAddress
)
/*++
Function Name:getMacAddress

Parameters:

Description:

Returns:

--*/
{
int i;
UINT fStatus;
int Size = 1024*5;
PNDIS_ENUM_INTF Interfaces;
UCHAR   OidVendData[16];

Interfaces = (PNDIS_ENUM_INTF) I_RpcAllocate (Size);
if (Interfaces == 0)
{
return FALSE;
}

if (NdisEnumerateInterfaces(Interfaces, Size))
{
UINT i;

for (i = 0; i < Interfaces->TotalInterfaces; i++)
{
PUNICODE_STRING pDeviceName= &(Interfaces->Interface[i].DeviceName);
UCHAR   PermMacAddr[6];

fStatus = NdisQueryHwAddress(pDeviceName, pMacAddress, PermMacAddr, 
&OidVendData[0]);
if (fStatus && (OidVendData[0] != 0xFF
|| OidVendData[1] != 0xFF
|| OidVendData[2] != 0xFF))
{
I_RpcFree (Interfaces);

return TRUE;
}
}
}

I_RpcFree (Interfaces);

return FALSE;
}

As see can see functions realted to UuidCreateSequential(it use the seed
ad the last 48 bits of uuid), the following code showed how Windows
obtain MAC address and OidVendorData, if OidVendorData's first 6 bytes
is 0xff, the function will fail, casing a random value generated rather
than the mac of our adapter. So I guess its related to the virtio
implementation. But I can't identify where the OidVendData is defined.
So I think I should file a issue to virtio dev teams too.

fStatus = NdisQueryHwAdd

Re: [PATCH] docs/devel: Document conventional file prefixes and suffixes

2023-12-26 Thread Zhao Liu
Hi Philippe,

On Tue, Dec 26, 2023 at 04:04:41PM +0100, Philippe Mathieu-Daudé wrote:
> Date: Tue, 26 Dec 2023 16:04:41 +0100
> From: Philippe Mathieu-Daudé 
> Subject: [PATCH] docs/devel: Document conventional file prefixes and
>  suffixes
> X-Mailer: git-send-email 2.41.0
> 
> Some header and source file names use common prefix / suffix
> but we never really ruled a convention. Start doing so with
> the current patterns from the tree.
> 
> Suggested-by: Alex Bennée 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  docs/devel/style.rst | 49 
>  1 file changed, 49 insertions(+)
> 
> diff --git a/docs/devel/style.rst b/docs/devel/style.rst
> index 2f68b50079..4da50eb2ea 100644
> --- a/docs/devel/style.rst
> +++ b/docs/devel/style.rst
> @@ -162,6 +162,55 @@ pre-processor. Another common suffix is ``_impl``; it is 
> used for the
>  concrete implementation of a function that will not be called
>  directly, but rather through a macro or an inline function.
>  
> +File Naming Conventions
> +---
> +
> +Public headers
> +~~
> +
> +Headers expected to be access by multiple subsystems must reside in
> +the ``include/`` folder. Headers local to a subsystem should reside in
> +the sysbsystem folder, if any (for example ``qobject/qobject-internal.h``
> +can only be included by files within the ``qobject/`` folder).
> +
> +Header file prefix and suffix hints
> +~~~
> +
> +When headers relate to common concept, it is useful to use a common
> +prefix or suffix.
> +
> +When headers relate to the same (guest) subsystem, the subsystem name is
> +often used as prefix. If headers are already in a folder named as the
> +subsystem, prefixing them is optional.
> +
> +For example, hardware models related to the Aspeed systems are named
> +using the ``aspeed_`` prefix.
> +
> +Headers related to the same (host) concept can also use a common prefix.
^^
 Maybe "suffix"?

since below you provide examples of "suffix".

> +For example OS specific headers use the ``-posix`` and ``-win32`` suffixes.
> +
> +Registered file suffixes
> +
> +
> +* ``.inc``
> +
> +  Source files meant to be included by other source files as templates
> +  must use the ``.c.inc`` suffix. Similarly, headers meant to be included
> +  multiple times as template must use the ``.h.inc`` suffix.
> +
> +Recommended file prefixes / suffixes
> +
> +
> +* ``target`` and ``common`` suffixes
> +
> +  Files which are specific to a target should use the ``target`` suffix.

emm, it seems linux-use/* and bsd-user/* have many ``target`` prefix
headers. Should they get cleaned up?


> +  Such ``target`` suffixed headers usually *taint* the files including them
> +  by making them target specific.
> +
> +  Files common to all targets should use the ``common`` suffix, to provide
> +  a hint that these files can be safely included from common code.
> +
> +

An additional question that kind of confuses me is whether header file
naming should use "-" or "_" to connect prefixes/suffixes?

>  Block structure
>  ===
>  
> -- 
> 2.41.0
> 

Thanks,
Zhao





Re: [External] Re: [PATCH v2 11/20] util/dsa: Implement DSA task asynchronous submission and wait for completion.

2023-12-26 Thread Hao Xiang
On Wed, Dec 13, 2023 at 6:01 AM Fabiano Rosas  wrote:
>
> Hao Xiang  writes:
>
> > * Add a DSA task completion callback.
> > * DSA completion thread will call the tasks's completion callback
> > on every task/batch task completion.
> > * DSA submission path to wait for completion.
> > * Implement CPU fallback if DSA is not able to complete the task.
> >
> > Signed-off-by: Hao Xiang 
> > Signed-off-by: Bryan Zhang 
> > ---
> >  include/qemu/dsa.h |  14 +
> >  util/dsa.c | 153 -
> >  2 files changed, 164 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/qemu/dsa.h b/include/qemu/dsa.h
> > index b10e7b8fb7..3f8ee07004 100644
> > --- a/include/qemu/dsa.h
> > +++ b/include/qemu/dsa.h
> > @@ -65,6 +65,20 @@ void buffer_zero_batch_task_init(struct 
> > buffer_zero_batch_task *task,
> >   */
> >  void buffer_zero_batch_task_destroy(struct buffer_zero_batch_task *task);
> >
> > +/**
> > + * @brief Performs buffer zero comparison on a DSA batch task 
> > asynchronously.
> > + *
> > + * @param batch_task A pointer to the batch task.
> > + * @param buf An array of memory buffers.
> > + * @param count The number of buffers in the array.
> > + * @param len The buffer length.
> > + *
> > + * @return Zero if successful, otherwise non-zero.
> > + */
> > +int
> > +buffer_is_zero_dsa_batch_async(struct buffer_zero_batch_task *batch_task,
> > +   const void **buf, size_t count, size_t len);
> > +
> >  /**
> >   * @brief Initializes DSA devices.
> >   *
> > diff --git a/util/dsa.c b/util/dsa.c
> > index 3cc017b8a0..06c6fbf2ca 100644
> > --- a/util/dsa.c
> > +++ b/util/dsa.c
> > @@ -470,6 +470,41 @@ poll_completion(struct dsa_completion_record 
> > *completion,
> >  return 0;
> >  }
> >
> > +/**
> > + * @brief Use CPU to complete a single zero page checking task.
> > + *
> > + * @param task A pointer to the task.
> > + */
> > +static void
> > +task_cpu_fallback(struct buffer_zero_batch_task *task)
> > +{
> > +assert(task->task_type == DSA_TASK);
> > +
> > +struct dsa_completion_record *completion = &task->completions[0];
> > +const uint8_t *buf;
> > +size_t len;
> > +
> > +if (completion->status == DSA_COMP_SUCCESS) {
> > +return;
> > +}
> > +
> > +/*
> > + * DSA was able to partially complete the operation. Check the
> > + * result. If we already know this is not a zero page, we can
> > + * return now.
> > + */
> > +if (completion->bytes_completed != 0 && completion->result != 0) {
> > +task->results[0] = false;
> > +return;
> > +}
> > +
> > +/* Let's fallback to use CPU to complete it. */
> > +buf = (const uint8_t *)task->descriptors[0].src_addr;
> > +len = task->descriptors[0].xfer_size;
> > +task->results[0] = buffer_is_zero(buf + completion->bytes_completed,
> > +  len - completion->bytes_completed);
> > +}
> > +
> >  /**
> >   * @brief Complete a single DSA task in the batch task.
> >   *
> > @@ -548,6 +583,62 @@ poll_batch_task_completion(struct 
> > buffer_zero_batch_task *batch_task)
> >  }
> >  }
> >
> > +/**
> > + * @brief Use CPU to complete the zero page checking batch task.
> > + *
> > + * @param batch_task A pointer to the batch task.
> > + */
> > +static void
> > +batch_task_cpu_fallback(struct buffer_zero_batch_task *batch_task)
> > +{
> > +assert(batch_task->task_type == DSA_BATCH_TASK);
> > +
> > +struct dsa_completion_record *batch_completion =
> > +&batch_task->batch_completion;
> > +struct dsa_completion_record *completion;
> > +uint8_t status;
> > +const uint8_t *buf;
> > +size_t len;
> > +bool *results = batch_task->results;
> > +uint32_t count = batch_task->batch_descriptor.desc_count;
> > +
> > +// DSA is able to complete the entire batch task.
> > +if (batch_completion->status == DSA_COMP_SUCCESS) {
> > +assert(count == batch_completion->bytes_completed);
> > +return;
> > +}
> > +
> > +/*
> > + * DSA encounters some error and is not able to complete
> > + * the entire batch task. Use CPU fallback.
> > + */
> > +for (int i = 0; i < count; i++) {
> > +completion = &batch_task->completions[i];
> > +status = completion->status;
> > +if (status == DSA_COMP_SUCCESS) {
> > +continue;
> > +}
> > +assert(status == DSA_COMP_PAGE_FAULT_NOBOF);
> > +
> > +/*
> > + * DSA was able to partially complete the operation. Check the
> > + * result. If we already know this is not a zero page, we can
> > + * return now.
> > + */
> > +if (completion->bytes_completed != 0 && completion->result != 0) {
> > +results[i] = false;
> > +continue;
> > +}
> > +
> > +/* Let's fallback to use CPU to complete it. */
> > +buf = (uint8_t *)batch_task->descriptors[i].src_addr;
> > +

Re: [External] Re: [PATCH v2 07/20] util/dsa: Implement DSA device start and stop logic.

2023-12-26 Thread Hao Xiang
On Tue, Dec 19, 2023 at 5:19 AM Fabiano Rosas  wrote:
>
> Hao Xiang  writes:
>
> >>
> >> > +}
> >> > +
> >> > +void dsa_start(void) {}
> >> > +
> >> > +void dsa_stop(void) {}
> >> > +
> >> > +void dsa_cleanup(void) {}
> >> > +
> >> > +#endif
> >>
> >> These could all be in the header.
> >
> > The function definitions are already in dsa.h Do you mean moving the
> > function implementations to the header as well?
> >
>
> I mean the empty !CONFIG_DSA_OPT variants could be in the header as
> static inline.
>

Fixed.



Re: [PATCH v4 2/4] Add RISC-V IOPMP support

2023-12-26 Thread Ethan Chen via
On Mon, Dec 18, 2023 at 02:04:06PM +1000, Alistair Francis wrote:
> On Wed, Nov 22, 2023 at 3:35 PM Ethan Chen via  wrote:
> >
> > Support specification Version 1.0.0-draft4 rapid-k model.
> > The specification url:
> > https://github.com/riscv-non-isa/iopmp-spec/blob/main/riscv_iopmp_specification.pdf
> >
> > IOPMP check memory access from deivce is valid or not. This implementation 
> > uses
> > IOMMU to change address space that device access. There are three possible
> > results of an access: valid, blocked, and stalled.
> >
> > If an access is valid, target address spcae is downstream_as(system_memory).
> > If an access is blocked, it will go to blocked_io_as. The operation of
> > blocked_io_as could be a bus error, a decode error, or it can respond a 
> > success
> > with fabricated data depending on IOPMP ERRREACT register value.
> > If an access is stalled, it will go to stall_io_as. The operation of 
> > stall_io_as
> > does nothing but return a stall result to source device. Source device 
> > should
> > retry the access if it gets a stall result.
> >
> > IOPMP implementation is rely on bus signal. For example IOPMP on AXI bus 
> > checks
> > the AXI burst transaction. A streamsink to receive general 
> > transaction_info(sid,
> > start address, end address) is added to IOPMP.
> >
> > If the source device support transaction_info, it can first send a
> > transaction_info to IOPMP streamsink then do the memory access. IOPMP will 
> > do
> > additional partially hit check with transaction info.
> > If the source device does not support transaction info. IOPMP will not check
> > partially hit.
> >
> > Signed-off-by: Ethan Chen 
> > ---
> >  hw/misc/Kconfig   |   4 +
> >  hw/misc/meson.build   |   1 +
> >  hw/misc/riscv_iopmp.c | 966 ++
> >  include/hw/misc/riscv_iopmp.h | 341 +++
> >  .../hw/misc/riscv_iopmp_transaction_info.h|  28 +
> >  5 files changed, 1340 insertions(+)
> >  create mode 100644 hw/misc/riscv_iopmp.c
> >  create mode 100644 include/hw/misc/riscv_iopmp.h
> >  create mode 100644 include/hw/misc/riscv_iopmp_transaction_info.h
> >
> > diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> > index cc8a8c1418..953569e682 100644
> > --- a/hw/misc/Kconfig
> > +++ b/hw/misc/Kconfig
> > @@ -200,4 +200,8 @@ config IOSB
> >  config XLNX_VERSAL_TRNG
> >  bool
> >
> > +config RISCV_IOPMP
> > +bool
> > +select STREAM
> > +
> >  source macio/Kconfig
> > diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> > index 36c20d5637..86b81e1690 100644
> > --- a/hw/misc/meson.build
> > +++ b/hw/misc/meson.build
> > @@ -35,6 +35,7 @@ system_ss.add(when: 'CONFIG_SIFIVE_E_PRCI', if_true: 
> > files('sifive_e_prci.c'))
> >  system_ss.add(when: 'CONFIG_SIFIVE_E_AON', if_true: 
> > files('sifive_e_aon.c'))
> >  system_ss.add(when: 'CONFIG_SIFIVE_U_OTP', if_true: 
> > files('sifive_u_otp.c'))
> >  system_ss.add(when: 'CONFIG_SIFIVE_U_PRCI', if_true: 
> > files('sifive_u_prci.c'))
> > +specific_ss.add(when: 'CONFIG_RISCV_IOPMP', if_true: 
> > files('riscv_iopmp.c'))
> >
> >  subdir('macio')
> >
> > diff --git a/hw/misc/riscv_iopmp.c b/hw/misc/riscv_iopmp.c
> > new file mode 100644
> > index 00..098a3b81fd
> > --- /dev/null
> > +++ b/hw/misc/riscv_iopmp.c
> > @@ -0,0 +1,966 @@
> > +/*
> > + * QEMU RISC-V IOPMP (Input Output Physical Memory Protection)
> > + *
> > + * Copyright (c) 2023 Andes Tech. Corp.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "qapi/error.h"
> > +#include "trace.h"
> > +#include "exec/exec-all.h"
> > +#include "exec/address-spaces.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/misc/riscv_iopmp.h"
> > +#include "memory.h"
> > +#include "hw/irq.h"
> > +
> > +#define TYPE_IOPMP_IOMMU_MEMORY_REGION "iopmp-iommu-memory-region"
> > +#define TYPE_IOPMP_TRASACTION_INFO_SINK "iopmp_transaction_info_sink"
> > +
> > +DECLARE_INSTANCE_CHECKER(Iopmp_StreamSink, IOPMP_TRASACTION_INFO_SINK,
> > + TYPE_IOPMP_TRASACTION_INFO_SINK)
> > +#define LOGGE(x...) qemu_log_mask(LOG_GUEST_ERROR, x)
> > +#define xLOG(x...)
> > +#define yLOG(x...) qemu_log(x)
> > +#ifdef DEBUG_RISCV_IOPMP
> > +  #define LOG(x...) yLOG(x)
> > +#e

Re: [External] Re: [PATCH v2 08/20] util/dsa: Implement DSA task enqueue and dequeue.

2023-12-26 Thread Hao Xiang
On Tue, Dec 12, 2023 at 8:10 AM Fabiano Rosas  wrote:
>
> Hao Xiang  writes:
>
> > * Use a safe thread queue for DSA task enqueue/dequeue.
> > * Implement DSA task submission.
> > * Implement DSA batch task submission.
> >
> > Signed-off-by: Hao Xiang 
> > ---
> >  include/qemu/dsa.h |  35 
> >  util/dsa.c | 196 +
> >  2 files changed, 231 insertions(+)
> >
> > diff --git a/include/qemu/dsa.h b/include/qemu/dsa.h
> > index 30246b507e..23f55185be 100644
> > --- a/include/qemu/dsa.h
> > +++ b/include/qemu/dsa.h
> > @@ -12,6 +12,41 @@
> >  #include 
> >  #include "x86intrin.h"
> >
> > +enum dsa_task_type {
>
> Our coding style requires CamelCase for enums and typedef'ed structures.

When I wrote this, I found numerous instances where snake case and no
typedef enum are used. But I do see the camel case and typedef'ed
instances now. Converted to that.

>
> > +DSA_TASK = 0,
> > +DSA_BATCH_TASK
> > +};
> > +
> > +enum dsa_task_status {
> > +DSA_TASK_READY = 0,
> > +DSA_TASK_PROCESSING,
> > +DSA_TASK_COMPLETION
> > +};
> > +
> > +typedef void (*buffer_zero_dsa_completion_fn)(void *);
>
> We don't really need the "buffer_zero" mention in any of this
> code. Simply dsa_batch_task or batch_task would suffice.

I removed "buffer_zero" prefix in some of the places.

>
> > +
> > +typedef struct buffer_zero_batch_task {
> > +struct dsa_hw_desc batch_descriptor;
> > +struct dsa_hw_desc *descriptors;
> > +struct dsa_completion_record batch_completion 
> > __attribute__((aligned(32)));
> > +struct dsa_completion_record *completions;
> > +struct dsa_device_group *group;
> > +struct dsa_device *device;
> > +buffer_zero_dsa_completion_fn completion_callback;
> > +QemuSemaphore sem_task_complete;
> > +enum dsa_task_type task_type;
> > +enum dsa_task_status status;
> > +bool *results;
> > +int batch_size;
> > +QSIMPLEQ_ENTRY(buffer_zero_batch_task) entry;
> > +} buffer_zero_batch_task;
>
> I see data specific to this implementation and data coming from the
> library, maybe these would be better organized in two separate
> structures with the qemu-specific having a pointer to the generic
> one. Looking ahead in the series, there seems to be migration data
> coming into this as well.

I refactored to create a generic structure batch_task and a DSA
specific version dsa_batch_task. batch_task has a pointer to
dsa_batch_task if DSA compilation option is enabled.

>
> > +
> > +#else
> > +
> > +struct buffer_zero_batch_task {
> > +bool *results;
> > +};
> > +
> >  #endif
> >
> >  /**
> > diff --git a/util/dsa.c b/util/dsa.c
> > index 8edaa892ec..f82282ce99 100644
> > --- a/util/dsa.c
> > +++ b/util/dsa.c
> > @@ -245,6 +245,200 @@ dsa_device_group_get_next_device(struct 
> > dsa_device_group *group)
> >  return &group->dsa_devices[current];
> >  }
> >
> > +/**
> > + * @brief Empties out the DSA task queue.
> > + *
> > + * @param group A pointer to the DSA device group.
> > + */
> > +static void
> > +dsa_empty_task_queue(struct dsa_device_group *group)
> > +{
> > +qemu_mutex_lock(&group->task_queue_lock);
> > +dsa_task_queue *task_queue = &group->task_queue;
> > +while (!QSIMPLEQ_EMPTY(task_queue)) {
> > +QSIMPLEQ_REMOVE_HEAD(task_queue, entry);
> > +}
> > +qemu_mutex_unlock(&group->task_queue_lock);
> > +}
> > +
> > +/**
> > + * @brief Adds a task to the DSA task queue.
> > + *
> > + * @param group A pointer to the DSA device group.
> > + * @param context A pointer to the DSA task to enqueue.
> > + *
> > + * @return int Zero if successful, otherwise a proper error code.
> > + */
> > +static int
> > +dsa_task_enqueue(struct dsa_device_group *group,
> > + struct buffer_zero_batch_task *task)
> > +{
> > +dsa_task_queue *task_queue = &group->task_queue;
> > +QemuMutex *task_queue_lock = &group->task_queue_lock;
> > +QemuCond *task_queue_cond = &group->task_queue_cond;
> > +
> > +bool notify = false;
> > +
> > +qemu_mutex_lock(task_queue_lock);
> > +
> > +if (!group->running) {
> > +fprintf(stderr, "DSA: Tried to queue task to stopped device 
> > queue\n");
> > +qemu_mutex_unlock(task_queue_lock);
> > +return -1;
> > +}
> > +
> > +// The queue is empty. This enqueue operation is a 0->1 transition.
> > +if (QSIMPLEQ_EMPTY(task_queue))
> > +notify = true;
> > +
> > +QSIMPLEQ_INSERT_TAIL(task_queue, task, entry);
> > +
> > +// We need to notify the waiter for 0->1 transitions.
> > +if (notify)
> > +qemu_cond_signal(task_queue_cond);
> > +
> > +qemu_mutex_unlock(task_queue_lock);
> > +
> > +return 0;
> > +}
> > +
> > +/**
> > + * @brief Takes a DSA task out of the task queue.
> > + *
> > + * @param group A pointer to the DSA device group.
> > + * @return buffer_zero_batch_task* The DSA task being dequeued.
> > + */
> > +__attribute__((unused))
> > +static struct buffer_z

Re: [PATCH v3 2/2] linux-user: Fix openat() emulation to not modify atime

2023-12-26 Thread Shu-Chun Weng
ping~

On Fri, Dec 8, 2023 at 2:42 PM Shu-Chun Weng  wrote:

> Commit b8002058 strengthened openat()'s /proc detection by calling
> realpath(3) on the given path, which allows various paths and symlinks
> that points to the /proc file system to be intercepted correctly.
>
> Using realpath(3), though, has a side effect that it reads the symlinks
> along the way, and thus changes their atime. The results in the
> following code snippet already get ~now instead of the real atime:
>
>   int fd = open("/path/to/a/symlink", O_PATH | O_NOFOLLOW);
>   struct stat st;
>   fstat(fd, st);
>   return st.st_atime;
>
> This change opens a path that doesn't appear to be part of /proc
> directly and checks the destination of /proc/self/fd/n to determine if
> it actually refers to a file in /proc.
>
> Neither this nor the existing code works with symlinks or indirect paths
> (e.g.  /tmp/../proc/self/exe) that points to /proc/self/exe because it
> is itself a symlink, and both realpath(3) and /proc/self/fd/n will
> resolve into the location of QEMU.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2004
> Signed-off-by: Shu-Chun Weng 
> ---
>  linux-user/syscall.c | 47 +++-
>  1 file changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e384e14248..7c3772301f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8308,8 +8308,7 @@ static int open_net_route(CPUArchState *cpu_env, int
> fd)
>  int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *fname,
>  int flags, mode_t mode, bool safe)
>  {
> -g_autofree char *proc_name = NULL;
> -const char *pathname;
> +g_autofree char *pathname = NULL;
>  struct fake_open {
>  const char *filename;
>  int (*fill)(CPUArchState *cpu_env, int fd);
> @@ -8334,12 +8333,42 @@ int do_guest_openat(CPUArchState *cpu_env, int
> dirfd, const char *fname,
>  { NULL, NULL, NULL }
>  };
>
> -/* if this is a file from /proc/ filesystem, expand full name */
> -proc_name = realpath(fname, NULL);
> -if (proc_name && strncmp(proc_name, "/proc/", 6) == 0) {
> -pathname = proc_name;
> +if (strncmp(fname, "/proc/", 6) == 0) {
> +pathname = g_strdup(fname);
>  } else {
> -pathname = fname;
> +g_autofree char *proc_name = NULL;
> +struct stat proc_stat;
> +int fd;
> +
> +if (safe) {
> +fd = safe_openat(dirfd, path(fname), flags, mode);
> +} else {
> +fd = openat(dirfd, path(fname), flags, mode);
> +}
> +if (fd < 0) {
> +return fd;
> +}
> +
> +/*
> + * Try to get the real path of the file we just opened. We avoid
> calling
> + * `realpath(3)` because it calls `readlink(2)` on symlinks which
> + * changes their atime. Note that since `/proc/self/exe` is a
> symlink,
> + * `pathname` will never resolve to it (neither will
> `realpath(3)`).
> + * That's why we check `fname` against the "/proc/" prefix first.
> + */
> +proc_name = g_strdup_printf("/proc/self/fd/%d", fd);
> +if (lstat(proc_name, &proc_stat) < 0 ||
> !S_ISLNK(proc_stat.st_mode)) {
> +/* No procfs or something weird. Not going to dig further. */
> +return fd;
> +}
> +pathname = g_new(char, proc_stat.st_size + 1);
> +readlink(proc_name, pathname, proc_stat.st_size + 1);
> +
> +/* if this is not a file from /proc/ filesystem, the fd is good
> as-is */
> +if (strncmp(pathname, "/proc/", 6) != 0) {
> +return fd;
> +}
> +close(fd);
>  }
>
>  if (is_proc_myself(pathname, "exe")) {
> @@ -8390,9 +8419,9 @@ int do_guest_openat(CPUArchState *cpu_env, int
> dirfd, const char *fname,
>  }
>
>  if (safe) {
> -return safe_openat(dirfd, path(pathname), flags, mode);
> +return safe_openat(dirfd, pathname, flags, mode);
>  } else {
> -return openat(dirfd, path(pathname), flags, mode);
> +return openat(dirfd, pathname, flags, mode);
>  }
>  }
>
>


smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PULL 0/7] Trivial patches for 2023-12-25

2023-12-26 Thread Michael Tokarev

26.12.2023 18:39, Philippe Mathieu-Daudé:
..

Stefan Weil via (1):
   virtio-blk: Fix potential nullpointer read access in 
virtio_blk_data_plane_destroy


This last patch has as author:

From: Stefan Weil via 


Ugh. It's too late. And yes, that's my mishap, - I forgot to
check for such stuff.  Sigh.  Will do the next time.

/mjt




Re: [PULL 0/1] Dirty page rate and dirty page limit 20231225 patch

2023-12-26 Thread Stefan Hajnoczi
Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any 
user-visible changes.


signature.asc
Description: PGP signature


Re: [PULL 0/7] Trivial patches for 2023-12-25

2023-12-26 Thread Stefan Hajnoczi
Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any 
user-visible changes.


signature.asc
Description: PGP signature


Re: [PULL 00/11] m68k next-cube patches

2023-12-26 Thread Stefan Hajnoczi
Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any 
user-visible changes.


signature.asc
Description: PGP signature


Re: [PULL 00/21] virtio,pc,pci: features, cleanups, fixes

2023-12-26 Thread Stefan Hajnoczi
Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any 
user-visible changes.


signature.asc
Description: PGP signature


Re: [RFC PATCH] meson.build: report graphics backends

2023-12-26 Thread Philippe Mathieu-Daudé

Hi Alex,

On 22/12/23 12:48, Alex Bennée wrote:

To enable accelerated VirtIO GPUs for the guest we need the rendering
support on the host but currently it's not reported in the
configuration summary. Add a graphics backend section and report the
status of the VirGL and Rutabaga support libraries.

Signed-off-by: Alex Bennée 
---
  meson.build | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/meson.build b/meson.build
index 6c77d9687de..93868568870 100644
--- a/meson.build
+++ b/meson.build
@@ -4307,6 +4307,12 @@ summary_info += {'curses support':curses}
  summary_info += {'brlapi support':brlapi}
  summary(summary_info, bool_yn: true, section: 'User interface')
  
+# Graphics backends

+summary_info = {}
+summary_info += {'VirGL support': virgl}
+summary_info += {'Rutabaga support':  rutabaga}


These are already under the generic 'Dependencies' section
below.


+summary(summary_info, bool_yn: true, section: 'Graphics backends')
+
  # Audio backends
  summary_info = {}
  if targetos not in ['darwin', 'haiku', 'windows']


Instead you want to move to your new section:

-- >8 --
diff --git a/meson.build b/meson.build
index 1bf526d653..ad7d870321 100644
--- a/meson.build
+++ b/meson.build
@@ -4308,4 +4308,11 @@ summary_info += {'brlapi support':brlapi}
 summary(summary_info, bool_yn: true, section: 'User interface')

+# Graphics backends
+summary_info = {}
+summary_info += {'OpenGL support (epoxy)': opengl}
+summary_info += {'VirGL support': virgl}
+summary_info += {'Rutabaga support': rutabaga}
+summary(summary_info, bool_yn: true, section: 'Graphics backends')
+
 # Audio backends
 summary_info = {}
@@ -4343,6 +4350,4 @@ summary_info += {'libtasn1':  tasn1}
 summary_info += {'PAM':   pam}
 summary_info += {'iconv support': iconv}
-summary_info += {'virgl support': virgl}
-summary_info += {'rutabaga support':  rutabaga}
 summary_info += {'blkio support': blkio}
 summary_info += {'curl support':  curl}
@@ -4361,5 +4366,4 @@ summary_info += {'U2F support':   u2f}
 summary_info += {'libusb':libusb}
 summary_info += {'usb net redir': usbredir}
-summary_info += {'OpenGL support (epoxy)': opengl}
 summary_info += {'GBM':   gbm}
 summary_info += {'libiscsi support':  libiscsi}
---

Without duplication:

Reviewed-by: Philippe Mathieu-Daudé 





Re: [PATCH 0/4] hw: Remove 'exec/exec-all.h' header when unused

2023-12-26 Thread Philippe Mathieu-Daudé

On 12/12/23 12:36, Philippe Mathieu-Daudé wrote:


Philippe Mathieu-Daudé (4):
   hw/ppc/spapr_hcall: Remove unused 'exec/exec-all.h' included header
   hw/misc/mips_itu: Remove unnecessary 'exec/exec-all.h' header
   hw/s390x/ipl: Remove unused 'exec/exec-all.h' included header
   target: Restrict 'sysemu/reset.h' to system emulation


Thanks Thomas for also merging these :)



Re: [PATCH 3/4] hw/s390x/ipl: Remove unused 'exec/exec-all.h' included header

2023-12-26 Thread Philippe Mathieu-Daudé

On 13/12/23 09:18, Christian Borntraeger wrote:

Am 12.12.23 um 16:28 schrieb Eric Farman:

So why do you think exec-all.h is unused?

I think because that got moved out of exec-all.h a few months ago, via

commit 3549118b498873c84b442bc280a5edafbb61e0a4
Author: Philippe Mathieu-Daudé 
Date:   Thu Sep 14 20:57:08 2023 +0200

 exec: Move cpu_loop_foo() target agnostic functions to 'cpu-
common.h'
 While these functions are not TCG specific, they are not target
 specific. Move them to "exec/cpu-common.h" so their callers don't
 have to be tainted as target specific.


Indeed, thanks Eric for justifying this patch.


Ah right, I was looking at an old QEMU version


FYI I tried to clarify a bit these header patterns here:
https://lore.kernel.org/qemu-devel/20231226150441.97501-1-phi...@linaro.org/



Re: [PULL 0/7] Trivial patches for 2023-12-25

2023-12-26 Thread Philippe Mathieu-Daudé

Hi,

On 25/12/23 09:10, Michael Tokarev wrote:



trivial patches for 2023-12-25




Akihiko Odaki (2):
   qemu-options: Unify the help entries for cocoa
   qemu-options: Tell more for -display cocoa

Elen Avan (1):
   include/ui/rect.h: fix qemu_rect_init() mis-assignment

Jai Arora (1):
   accel/kvm: Turn DPRINTF macro use into tracepoints

Natanael Copa (1):
   target/riscv/kvm: do not use non-portable strerrorname_np()

Samuel Tardieu (1):
   docs/tools/qemu-img.rst: fix typo (sumarizes)

Stefan Weil via (1):
   virtio-blk: Fix potential nullpointer read access in 
virtio_blk_data_plane_destroy


This last patch has as author:

From: Stefan Weil via 




Re: [PATCH 0/2] system/qtest: Minor include cleanups

2023-12-26 Thread Philippe Mathieu-Daudé

On 12/12/23 12:30, Philippe Mathieu-Daudé wrote:

Add missing header and restrict to sysemu.

Philippe Mathieu-Daudé (2):
   system/qtest: Include missing 'hw/core/cpu.h' header
   system/qtest: Restrict QTest API to system emulation


Thanks Thomas for merging these!




Re: [PATCH] virtio-blk: Fix potential nullpointer read access in virtio_blk_data_plane_destroy

2023-12-26 Thread Philippe Mathieu-Daudé

On 24/12/23 12:43, Stefan Weil via wrote:

Fixes: CID 1532828
Fixes: b6948ab01d ("virtio-blk: add iothread-vq-mapping parameter")
Signed-off-by: Stefan Weil 
---
  hw/block/dataplane/virtio-blk.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 23/24] exec/cpu-all: Extract page-protection definitions to page-prot-common.h

2023-12-26 Thread Philippe Mathieu-Daudé

Hi Nicholas,

On 22/12/23 09:06, Nicholas Piggin wrote:

On Tue Dec 12, 2023 at 7:20 AM AEST, Philippe Mathieu-Daudé wrote:

Extract page-protection definitions from "exec/cpu-all.h"
to "exec/page-prot-common.h".

The list of files requiring the new header was generated
using:

$ git grep -wE \
   'PAGE_(READ|WRITE|EXEC|BITS|VALID|ANON|RESERVED|TARGET_.|PASSTHROUGH)'


Acked-by: Nicholas Piggin  (ppc)

Looks trivial for ppc, so fine. But what's the difference between
-common.h and -all.h suffix?


We believe historically '-all' was used for target-specific
headers. However then headers got massaged and some ended
using this suffix, although not anymore target specific.

Today for clarity we prefer using '-common' for generic
(target agnostic) headers, and '-target' for target-specific
ones.

I tried to clarify that here:
https://lore.kernel.org/qemu-devel/20231226150441.97501-1-phi...@linaro.org/

Thanks for your review,

Phil.



[PATCH] docs/devel: Document conventional file prefixes and suffixes

2023-12-26 Thread Philippe Mathieu-Daudé
Some header and source file names use common prefix / suffix
but we never really ruled a convention. Start doing so with
the current patterns from the tree.

Suggested-by: Alex Bennée 
Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/devel/style.rst | 49 
 1 file changed, 49 insertions(+)

diff --git a/docs/devel/style.rst b/docs/devel/style.rst
index 2f68b50079..4da50eb2ea 100644
--- a/docs/devel/style.rst
+++ b/docs/devel/style.rst
@@ -162,6 +162,55 @@ pre-processor. Another common suffix is ``_impl``; it is 
used for the
 concrete implementation of a function that will not be called
 directly, but rather through a macro or an inline function.
 
+File Naming Conventions
+---
+
+Public headers
+~~
+
+Headers expected to be access by multiple subsystems must reside in
+the ``include/`` folder. Headers local to a subsystem should reside in
+the sysbsystem folder, if any (for example ``qobject/qobject-internal.h``
+can only be included by files within the ``qobject/`` folder).
+
+Header file prefix and suffix hints
+~~~
+
+When headers relate to common concept, it is useful to use a common
+prefix or suffix.
+
+When headers relate to the same (guest) subsystem, the subsystem name is
+often used as prefix. If headers are already in a folder named as the
+subsystem, prefixing them is optional.
+
+For example, hardware models related to the Aspeed systems are named
+using the ``aspeed_`` prefix.
+
+Headers related to the same (host) concept can also use a common prefix.
+For example OS specific headers use the ``-posix`` and ``-win32`` suffixes.
+
+Registered file suffixes
+
+
+* ``.inc``
+
+  Source files meant to be included by other source files as templates
+  must use the ``.c.inc`` suffix. Similarly, headers meant to be included
+  multiple times as template must use the ``.h.inc`` suffix.
+
+Recommended file prefixes / suffixes
+
+
+* ``target`` and ``common`` suffixes
+
+  Files which are specific to a target should use the ``target`` suffix.
+  Such ``target`` suffixed headers usually *taint* the files including them
+  by making them target specific.
+
+  Files common to all targets should use the ``common`` suffix, to provide
+  a hint that these files can be safely included from common code.
+
+
 Block structure
 ===
 
-- 
2.41.0




Re: [PATCH] esp: process the result of scsi_device_find()

2023-12-26 Thread Philippe Mathieu-Daudé

Cc'ing Mark for the logical change (should we rather assert?).

On 18/12/23 16:02, Alexandra Diupina wrote:

Add a 'current_lun' check for a null value
to avoid null pointer dereferencing

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 4eb8606560 (esp: store lun coming from the MESSAGE OUT phase)
Signed-off-by: Alexandra Diupina 
---
  hw/scsi/esp.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 9b11d8c573..3a2ec35f9b 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -292,6 +292,11 @@ static void do_command_phase(ESPState *s)
  esp_fifo_pop_buf(&s->cmdfifo, buf, cmdlen);
  
  current_lun = scsi_device_find(&s->bus, 0, s->current_dev->id, s->lun);

+
+if (!current_lun) {
+return;
+}
+
  s->current_req = scsi_req_new(current_lun, 0, s->lun, buf, cmdlen, s);
  datalen = scsi_req_enqueue(s->current_req);
  s->ti_size = datalen;





Re: [PATCH] load_elf: fix iterator type in glue

2023-12-26 Thread Philippe Mathieu-Daudé

Hi,

On 21/12/23 09:08, Anastasia Belova wrote:

file_size is uint32_t, so j < file_size should be
uint32_t too.


file_size is of elf_word type, which is either uint32_t
or uint64_t.

Your explanation is not very clear... Maybe you want an unsigned type?
In that case, does the following makes your sanitizer happier?

-- >8 --
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 0a5c258fe6..03eba78c6e 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -502,4 +502,3 @@ static ssize_t glue(load_elf, SZ)(const char *name, 
int fd,

 if (data_swab) {
-int j;
-for (j = 0; j < file_size; j += (1 << data_swab)) {
+for (unsigned j = 0; j < file_size; j += (1 << 
data_swab)) {

 uint8_t *dp = data + j;
---


Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 7ef295ea5b ("loader: Add data swap option to load-elf")
Signed-off-by: Anastasia Belova 
---
  include/hw/elf_ops.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 0a5c258fe6..1defccaa71 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -500,7 +500,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
  }
  
  if (data_swab) {

-int j;
+uint32_t j;
  for (j = 0; j < file_size; j += (1 << data_swab)) {
  uint8_t *dp = data + j;
  switch (data_swab) {





Re: [PATCH] hw/m68k/mcf5206: Embed m5206_timer_state in m5206_mbar_state

2023-12-26 Thread Philippe Mathieu-Daudé

On 21/12/23 13:29, Thomas Huth wrote:

There's no need to explicitely allocate the memory here, we can
simply embed it into the m5206_mbar_state instead.

Signed-off-by: Thomas Huth 
---
  hw/m68k/mcf5206.c | 20 
  1 file changed, 8 insertions(+), 12 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH] include/ui/rect.h: fix qemu_rect_init() mis-assignment

2023-12-26 Thread Philippe Mathieu-Daudé

On 22/12/23 20:17, Michael Tokarev wrote:

From: Elen Avan 
Signed-off-by: Elen Avan 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2051
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2050
Fixes: a200d53b1fde "virtio-gpu: replace PIXMAN for region/rect test"
Cc: qemu-sta...@nongnu.org
Reviewed-by: Michael Tokarev 
---
  include/ui/rect.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 00/33] hw/cpu/arm: Remove one use of qemu_get_cpu() in A7/A15 MPCore priv

2023-12-26 Thread Philippe Mathieu-Daudé

Hi,

ping for review?

On 12/12/23 17:29, Philippe Mathieu-Daudé wrote:

Hi,

When a MPCore cluster is used, the Cortex-A cores belong the the
cluster container, not to the board/soc layer. This series move
the creation of vCPUs to the MPCore private container.

Doing so we consolidate the QOM model, moving common code in a
central place (abstract MPCore parent).

This eventually allow removing one qemu_get_cpu() use, which we
want to remove in heterogeneous machines (machines using MPCore
are candidate for heterogeneous emulation).

Maybe these hw/cpu/arm/ files belong to hw/arm/...

Regards,

Phil.

Philippe Mathieu-Daudé (33):
   hw/arm/boot: Propagate vCPU to arm_load_dtb()
   hw/arm/fsl-imx6: Add a local 'gic' variable
   hw/arm/fsl-imx6ul: Add a local 'gic' variable
   hw/arm/fsl-imx7: Add a local 'gic' variable
   hw/cpu: Remove dead Kconfig
   hw/cpu/arm: Rename 'busdev' -> 'gicsbd' in a15mp_priv_realize()
   hw/cpu/arm: Alias 'num-cpu' property on TYPE_REALVIEW_MPCORE
   hw/cpu/arm: Declare CPU QOM types using DEFINE_TYPES() macro
   hw/cpu/arm: Merge {a9mpcore.h, a15mpcore.h} as cortex_mpcore.h
   hw/cpu/arm: Introduce abstract CORTEX_MPCORE_PRIV QOM type
   hw/cpu/arm: Have A9MPCORE/A15MPCORE inheritate common
 CORTEX_MPCORE_PRIV
   hw/cpu/arm: Create MPCore container in QOM parent
   hw/cpu/arm: Handle 'num_cores' property once in MPCore parent
   hw/cpu/arm: Handle 'has_el2/3' properties once in MPCore parent
   hw/cpu/arm: Handle 'gic-irq' property once in MPCore parent
   hw/cpu/arm: Handle GIC once in MPCore parent
   hw/cpu/arm: Document more properties of CORTEX_MPCORE_PRIV QOM type
   hw/cpu/arm: Replace A15MPPrivState by CortexMPPrivState
   hw/cpu/arm: Introduce TYPE_A7MPCORE_PRIV for Cortex-A7 MPCore
   hw/cpu/arm: Consolidate check on max GIC spi supported
   hw/cpu/arm: Create CPUs once in MPCore parent
   hw/arm/aspeed_ast2600: Let the A7MPcore create/wire the CPU cores
   hw/arm/exynos4210: Let the A9MPcore create/wire the CPU cores
   hw/arm/fsl-imx6: Let the A9MPcore create/wire the CPU cores
   hw/arm/fsl-imx6ul: Let the A7MPcore create/wire the CPU cores
   hw/arm/fsl-imx7: Let the A7MPcore create/wire the CPU cores
   hw/arm/highbank: Let the A9/A15MPcore create/wire the CPU cores
   hw/arm/vexpress: Let the A9/A15MPcore create/wire the CPU cores
   hw/arm/xilinx_zynq: Let the A9MPcore create/wire the CPU cores
   hw/arm/npcm7xx: Let the A9MPcore create/wire the CPU cores
   hw/cpu/a9mpcore: Remove legacy code
   hw/cpu/arm: Remove 'num-cpu' property alias
   hw/cpu/arm: Remove use of qemu_get_cpu() in A7/A15 realize()





Re: [PATCH 0/2] target/alpha: Only build sys_helper.c on system emulation

2023-12-26 Thread Philippe Mathieu-Daudé

On 7/12/23 11:54, Philippe Mathieu-Daudé wrote:

Extract helper_load_pcc() to clk_helper.c so we can
restrict sys_helper.c to system emulation.

Philippe Mathieu-Daudé (2):
   target/alpha: Extract clk_helper.c from sys_helper.c
   target/alpha: Only build sys_helper.c on system emulation

  target/alpha/clk_helper.c | 32 
  target/alpha/sys_helper.c | 18 --
  target/alpha/meson.build  |  7 +--
  3 files changed, 37 insertions(+), 20 deletions(-)
  create mode 100644 target/alpha/clk_helper.c


Reviewed-by: Philippe Mathieu-Daudé 





Re: [PULL 00/21] virtio,pc,pci: features, cleanups, fixes

2023-12-26 Thread Michael S. Tsirkin
On Tue, Dec 26, 2023 at 04:24:01AM -0500, Michael S. Tsirkin wrote:
> The following changes since commit 80f1709aa0eb4de09b4240563463f991a5b9d855:
> 
>   Merge tag 'pull-loongarch-20231221' of https://gitlab.com/gaosong/qemu into 
> staging (2023-12-21 19:44:19 -0500)
> 
> are available in the Git repository at:
> 
>   https://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> 
> for you to fetch changes up to 7b67b2f0f4f7c5ec888a331af599d9daff735d60:

f6fe3e333fe0fcb8ef87c669a3a8f84fbee10cb7 now - fixed one of commit logs.


>   vdpa: move memory listener to vhost_vdpa_shared (2023-12-25 11:34:55 -0500)
> 
> 
> virtio,pc,pci: features, cleanups, fixes
> 
> vhost-scsi support for worker ioctls
> 
> fixes, cleanups all over the place.
> 
> Signed-off-by: Michael S. Tsirkin 
> 
> 
> Aaron Young (1):
>   hw/acpi: propagate vcpu hotplug after switch to modern interface
> 
> Dongli Zhang (1):
>   vhost-scsi: fix usage of error_reportf_err()
> 
> Eugenio Pérez (14):
>   vdpa: do not set virtio status bits if unneeded
>   vdpa: add VhostVDPAShared
>   vdpa: move iova tree to the shared struct
>   vdpa: move iova_range to vhost_vdpa_shared
>   vdpa: move shadow_data to vhost_vdpa_shared
>   vdpa: use vdpa shared for tracing
>   vdpa: move file descriptor to vhost_vdpa_shared
>   vdpa: move iotlb_batch_begin_sent to vhost_vdpa_shared
>   vdpa: move backend_cap to vhost_vdpa_shared
>   vdpa: remove msg type of vhost_vdpa
>   vdpa: move iommu_list to vhost_vdpa_shared
>   vdpa: use VhostVDPAShared in vdpa_dma_map and unmap
>   vdpa: use dev_shared in vdpa_iommu
>   vdpa: move memory listener to vhost_vdpa_shared
> 
> Mathieu Poirier (1):
>   virtio: rng: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX
> 
> Mike Christie (2):
>   vhost: Add worker backend callouts
>   vhost-scsi: Add support for a worker thread per virtqueue
> 
> Zhao Liu (1):
>   tests: bios-tables-test: Rename smbios type 4 related test functions
> 
> wangmeiling (1):
>   Fix bugs when VM shutdown with virtio-gpu unplugged
> 
>  include/hw/virtio/vhost-backend.h |  14 
>  include/hw/virtio/vhost-vdpa.h|  40 ++
>  include/hw/virtio/virtio-scsi.h   |   1 +
>  hw/acpi/cpu_hotplug.c |  20 -
>  hw/display/virtio-gpu-base.c  |   4 +
>  hw/scsi/vhost-scsi.c  |  66 ++-
>  hw/scsi/vhost-user-scsi.c |   3 +-
>  hw/virtio/vdpa-dev.c  |   7 +-
>  hw/virtio/vhost-backend.c |  28 +++
>  hw/virtio/vhost-user-rng.c|  16 
>  hw/virtio/vhost-vdpa.c| 164 
> --
>  net/vhost-vdpa.c  | 116 +--
>  tests/qtest/bios-tables-test.c|  20 ++---
>  hw/virtio/trace-events|  14 ++--
>  14 files changed, 334 insertions(+), 179 deletions(-)
> 




[PULL 06/21] vhost-scsi: fix usage of error_reportf_err()

2023-12-26 Thread Michael S. Tsirkin
From: Dongli Zhang 

It is required to use error_report() instead of error_reportf_err(), if the
prior function does not take local_err as the argument. As a result, the
local_err is always NULL and segment fault may happen.

vhost_scsi_start()
-> vhost_scsi_set_endpoint(s) --> does not allocate local_err
-> error_reportf_err()
   -> error_vprepend()
  -> g_string_append(newmsg, (*errp)->msg) --> (*errp) is NULL

In addition, add ": " at the end of other error_reportf_err() logs.

Fixes: 7962e432b4e4 ("vhost-user-scsi: support reconnect to backend")
Signed-off-by: Dongli Zhang 
Message-Id: <20231214003117.43960-1-dongli.zh...@oracle.com>
Reviewed-by: Feng Li 
Reviewed-by: Raphael Norwitz 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/scsi/vhost-scsi.c  | 4 ++--
 hw/scsi/vhost-user-scsi.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 08aa7534df..6159eb6fec 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -91,13 +91,13 @@ static int vhost_scsi_start(VHostSCSI *s)
 
 ret = vhost_scsi_common_start(vsc, &local_err);
 if (ret < 0) {
-error_reportf_err(local_err, "Error starting vhost-scsi");
+error_reportf_err(local_err, "Error starting vhost-scsi: ");
 return ret;
 }
 
 ret = vhost_scsi_set_endpoint(s);
 if (ret < 0) {
-error_reportf_err(local_err, "Error setting vhost-scsi endpoint");
+error_report("Error setting vhost-scsi endpoint");
 vhost_scsi_common_stop(vsc);
 }
 
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 780f10559d..af18c4f3d3 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -83,7 +83,8 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, 
uint8_t status)
 if (should_start) {
 ret = vhost_user_scsi_start(s, &local_err);
 if (ret < 0) {
-error_reportf_err(local_err, "unable to start vhost-user-scsi: %s",
+error_reportf_err(local_err,
+  "unable to start vhost-user-scsi: %s: ",
   strerror(-ret));
 qemu_chr_fe_disconnect(&vs->conf.chardev);
 }
-- 
MST




[PULL 14/21] vdpa: move file descriptor to vhost_vdpa_shared

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next patches will register the vhost_vdpa memory listener while the VM
is migrating at the destination, so we can map the memory to the device
before stopping the VM at the source.  The main goal is to reduce the
downtime.

However, the destination QEMU is unaware of which vhost_vdpa device will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the CVQ device.  Otherwise, it  will be the first one.

Move the file descriptor to VhostVDPAShared so all vhost_vdpa can use
it, rather than always in the first / last vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-7-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h |  2 +-
 hw/virtio/vdpa-dev.c   |  2 +-
 hw/virtio/vhost-vdpa.c | 14 +++---
 net/vhost-vdpa.c   | 11 ---
 4 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 01e0f25e27..796a180afa 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -32,6 +32,7 @@ typedef struct VhostVDPAHostNotifier {
 
 /* Info shared by all vhost_vdpa device models */
 typedef struct vhost_vdpa_shared {
+int device_fd;
 struct vhost_vdpa_iova_range iova_range;
 
 /* IOVA mapping used by the Shadow Virtqueue */
@@ -42,7 +43,6 @@ typedef struct vhost_vdpa_shared {
 } VhostVDPAShared;
 
 typedef struct vhost_vdpa {
-int device_fd;
 int index;
 uint32_t msg_type;
 bool iotlb_batch_begin_sent;
diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
index 457960d28a..8774986571 100644
--- a/hw/virtio/vdpa-dev.c
+++ b/hw/virtio/vdpa-dev.c
@@ -66,7 +66,6 @@ static void vhost_vdpa_device_realize(DeviceState *dev, Error 
**errp)
 if (*errp) {
 return;
 }
-v->vdpa.device_fd = v->vhostfd;
 
 v->vdev_id = vhost_vdpa_device_get_u32(v->vhostfd,
VHOST_VDPA_GET_DEVICE_ID, errp);
@@ -115,6 +114,7 @@ static void vhost_vdpa_device_realize(DeviceState *dev, 
Error **errp)
 goto free_vqs;
 }
 v->vdpa.shared = g_new0(VhostVDPAShared, 1);
+v->vdpa.shared->device_fd = v->vhostfd;
 v->vdpa.shared->iova_range = iova_range;
 
 ret = vhost_dev_init(&v->dev, &v->vdpa, VHOST_BACKEND_TYPE_VDPA, 0, NULL);
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 720cffbc08..09df150ef2 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -90,7 +90,7 @@ int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t asid, 
hwaddr iova,
hwaddr size, void *vaddr, bool readonly)
 {
 struct vhost_msg_v2 msg = {};
-int fd = v->device_fd;
+int fd = v->shared->device_fd;
 int ret = 0;
 
 msg.type = v->msg_type;
@@ -122,7 +122,7 @@ int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t 
asid, hwaddr iova,
  hwaddr size)
 {
 struct vhost_msg_v2 msg = {};
-int fd = v->device_fd;
+int fd = v->shared->device_fd;
 int ret = 0;
 
 msg.type = v->msg_type;
@@ -145,7 +145,7 @@ int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t 
asid, hwaddr iova,
 
 static void vhost_vdpa_listener_begin_batch(struct vhost_vdpa *v)
 {
-int fd = v->device_fd;
+int fd = v->shared->device_fd;
 struct vhost_msg_v2 msg = {
 .type = v->msg_type,
 .iotlb.type = VHOST_IOTLB_BATCH_BEGIN,
@@ -174,7 +174,7 @@ static void vhost_vdpa_listener_commit(MemoryListener 
*listener)
 struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
 struct vhost_dev *dev = v->dev;
 struct vhost_msg_v2 msg = {};
-int fd = v->device_fd;
+int fd = v->shared->device_fd;
 
 if (!(dev->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) {
 return;
@@ -499,7 +499,7 @@ static int vhost_vdpa_call(struct vhost_dev *dev, unsigned 
long int request,
  void *arg)
 {
 struct vhost_vdpa *v = dev->opaque;
-int fd = v->device_fd;
+int fd = v->shared->device_fd;
 int ret;
 
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA);
@@ -661,7 +661,7 @@ static int vhost_vdpa_host_notifier_init(struct vhost_dev 
*dev, int queue_index)
 struct vhost_vdpa *v = dev->opaque;
 VirtIODevice *vdev = dev->vdev;
 VhostVDPAHostNotifier *n;
-int fd = v->device_fd;
+int fd = v->shared->device_fd;
 void *addr;
 char *name;
 
@@ -1290,7 +1290,7 @@ static void vhost_vdpa_suspend(struct vhost_dev *dev)
 
 if (dev->backend_cap & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) {
 trace_vhost_vdpa_suspend(dev);
-r = ioctl(v->device_fd, VHOST_VDPA_SUSPEND);
+r = ioctl(v->shared->device_fd, VHOST_VDPA_SUSPEND);
 if (unlikely(r)) {
 error_report("Cannot suspend: %s(%d)", g_strerror(errno), errno

[PULL 15/21] vdpa: move iotlb_batch_begin_sent to vhost_vdpa_shared

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next patches will register the vhost_vdpa memory listener while the VM
is migrating at the destination, so we can map the memory to the device
before stopping the VM at the source.  The main goal is to reduce the
downtime.

However, the destination QEMU is unaware of which vhost_vdpa device will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the CVQ device.  Otherwise, it  will be the first one.

Move the iotlb_batch_begin_sent member to VhostVDPAShared so all
vhost_vdpa can use it, rather than always in the first / last
vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-8-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h | 3 ++-
 hw/virtio/vhost-vdpa.c | 8 
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 796a180afa..05219bbcf7 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -38,6 +38,8 @@ typedef struct vhost_vdpa_shared {
 /* IOVA mapping used by the Shadow Virtqueue */
 VhostIOVATree *iova_tree;
 
+bool iotlb_batch_begin_sent;
+
 /* Vdpa must send shadow addresses as IOTLB key for data queues, not GPA */
 bool shadow_data;
 } VhostVDPAShared;
@@ -45,7 +47,6 @@ typedef struct vhost_vdpa_shared {
 typedef struct vhost_vdpa {
 int index;
 uint32_t msg_type;
-bool iotlb_batch_begin_sent;
 uint32_t address_space_id;
 MemoryListener listener;
 uint64_t acked_features;
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 09df150ef2..2ecaedb686 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -162,11 +162,11 @@ static void vhost_vdpa_listener_begin_batch(struct 
vhost_vdpa *v)
 static void vhost_vdpa_iotlb_batch_begin_once(struct vhost_vdpa *v)
 {
 if (v->dev->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH) &&
-!v->iotlb_batch_begin_sent) {
+!v->shared->iotlb_batch_begin_sent) {
 vhost_vdpa_listener_begin_batch(v);
 }
 
-v->iotlb_batch_begin_sent = true;
+v->shared->iotlb_batch_begin_sent = true;
 }
 
 static void vhost_vdpa_listener_commit(MemoryListener *listener)
@@ -180,7 +180,7 @@ static void vhost_vdpa_listener_commit(MemoryListener 
*listener)
 return;
 }
 
-if (!v->iotlb_batch_begin_sent) {
+if (!v->shared->iotlb_batch_begin_sent) {
 return;
 }
 
@@ -193,7 +193,7 @@ static void vhost_vdpa_listener_commit(MemoryListener 
*listener)
  fd, errno, strerror(errno));
 }
 
-v->iotlb_batch_begin_sent = false;
+v->shared->iotlb_batch_begin_sent = false;
 }
 
 static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
-- 
MST




[PULL 19/21] vdpa: use VhostVDPAShared in vdpa_dma_map and unmap

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

The callers only have the shared information by the end of this series.
Start converting this functions.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-12-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h |  4 +--
 hw/virtio/vhost-vdpa.c | 50 +-
 net/vhost-vdpa.c   |  5 ++--
 3 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 3880b9e7f2..705c754776 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -69,9 +69,9 @@ typedef struct vhost_vdpa {
 int vhost_vdpa_get_iova_range(int fd, struct vhost_vdpa_iova_range 
*iova_range);
 int vhost_vdpa_set_vring_ready(struct vhost_vdpa *v, unsigned idx);
 
-int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t asid, hwaddr iova,
+int vhost_vdpa_dma_map(VhostVDPAShared *s, uint32_t asid, hwaddr iova,
hwaddr size, void *vaddr, bool readonly);
-int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t asid, hwaddr iova,
+int vhost_vdpa_dma_unmap(VhostVDPAShared *s, uint32_t asid, hwaddr iova,
  hwaddr size);
 
 typedef struct vdpa_iommu {
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index a2713b9f0b..ada3cc5d7c 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -86,11 +86,11 @@ static bool 
vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
  * The caller must set asid = 0 if the device does not support asid.
  * This is not an ABI break since it is set to 0 by the initializer anyway.
  */
-int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t asid, hwaddr iova,
+int vhost_vdpa_dma_map(VhostVDPAShared *s, uint32_t asid, hwaddr iova,
hwaddr size, void *vaddr, bool readonly)
 {
 struct vhost_msg_v2 msg = {};
-int fd = v->shared->device_fd;
+int fd = s->device_fd;
 int ret = 0;
 
 msg.type = VHOST_IOTLB_MSG_V2;
@@ -101,7 +101,7 @@ int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t asid, 
hwaddr iova,
 msg.iotlb.perm = readonly ? VHOST_ACCESS_RO : VHOST_ACCESS_RW;
 msg.iotlb.type = VHOST_IOTLB_UPDATE;
 
-trace_vhost_vdpa_dma_map(v->shared, fd, msg.type, msg.asid, msg.iotlb.iova,
+trace_vhost_vdpa_dma_map(s, fd, msg.type, msg.asid, msg.iotlb.iova,
  msg.iotlb.size, msg.iotlb.uaddr, msg.iotlb.perm,
  msg.iotlb.type);
 
@@ -118,11 +118,11 @@ int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t 
asid, hwaddr iova,
  * The caller must set asid = 0 if the device does not support asid.
  * This is not an ABI break since it is set to 0 by the initializer anyway.
  */
-int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t asid, hwaddr iova,
+int vhost_vdpa_dma_unmap(VhostVDPAShared *s, uint32_t asid, hwaddr iova,
  hwaddr size)
 {
 struct vhost_msg_v2 msg = {};
-int fd = v->shared->device_fd;
+int fd = s->device_fd;
 int ret = 0;
 
 msg.type = VHOST_IOTLB_MSG_V2;
@@ -131,8 +131,8 @@ int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t 
asid, hwaddr iova,
 msg.iotlb.size = size;
 msg.iotlb.type = VHOST_IOTLB_INVALIDATE;
 
-trace_vhost_vdpa_dma_unmap(v->shared, fd, msg.type, msg.asid,
-   msg.iotlb.iova, msg.iotlb.size, msg.iotlb.type);
+trace_vhost_vdpa_dma_unmap(s, fd, msg.type, msg.asid, msg.iotlb.iova,
+   msg.iotlb.size, msg.iotlb.type);
 
 if (write(fd, &msg, sizeof(msg)) != sizeof(msg)) {
 error_report("failed to write, fd=%d, errno=%d (%s)",
@@ -143,30 +143,29 @@ int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t 
asid, hwaddr iova,
 return ret;
 }
 
-static void vhost_vdpa_listener_begin_batch(struct vhost_vdpa *v)
+static void vhost_vdpa_listener_begin_batch(VhostVDPAShared *s)
 {
-int fd = v->shared->device_fd;
+int fd = s->device_fd;
 struct vhost_msg_v2 msg = {
 .type = VHOST_IOTLB_MSG_V2,
 .iotlb.type = VHOST_IOTLB_BATCH_BEGIN,
 };
 
-trace_vhost_vdpa_listener_begin_batch(v->shared, fd, msg.type,
-  msg.iotlb.type);
+trace_vhost_vdpa_listener_begin_batch(s, fd, msg.type, msg.iotlb.type);
 if (write(fd, &msg, sizeof(msg)) != sizeof(msg)) {
 error_report("failed to write, fd=%d, errno=%d (%s)",
  fd, errno, strerror(errno));
 }
 }
 
-static void vhost_vdpa_iotlb_batch_begin_once(struct vhost_vdpa *v)
+static void vhost_vdpa_iotlb_batch_begin_once(VhostVDPAShared *s)
 {
-if (v->shared->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH) &&
-!v->shared->iotlb_batch_begin_sent) {
-vhost_vdpa_listener_begin_batch(v);
+if (s->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTL

[PULL 12/21] vdpa: move shadow_data to vhost_vdpa_shared

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next patches will register the vhost_vdpa memory listener while the VM
is migrating at the destination, so we can map the memory to the device
before stopping the VM at the source.  The main goal is to reduce the
downtime.

However, the destination QEMU is unaware of which vhost_vdpa device will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the CVQ device.  Otherwise, it  will be the first one.

Move the shadow_data member to VhostVDPAShared so all vhost_vdpa can use
it, rather than always in the first or last vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-5-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h |  5 +++--
 hw/virtio/vhost-vdpa.c |  6 +++---
 net/vhost-vdpa.c   | 22 +-
 3 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 8d52a7e498..01e0f25e27 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -36,6 +36,9 @@ typedef struct vhost_vdpa_shared {
 
 /* IOVA mapping used by the Shadow Virtqueue */
 VhostIOVATree *iova_tree;
+
+/* Vdpa must send shadow addresses as IOTLB key for data queues, not GPA */
+bool shadow_data;
 } VhostVDPAShared;
 
 typedef struct vhost_vdpa {
@@ -47,8 +50,6 @@ typedef struct vhost_vdpa {
 MemoryListener listener;
 uint64_t acked_features;
 bool shadow_vqs_enabled;
-/* Vdpa must send shadow addresses as IOTLB key for data queues, not GPA */
-bool shadow_data;
 /* Device suspended successfully */
 bool suspended;
 VhostVDPAShared *shared;
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 5ff1d43ba9..97588848fc 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -353,7 +353,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
  vaddr, section->readonly);
 
 llsize = int128_sub(llend, int128_make64(iova));
-if (v->shadow_data) {
+if (v->shared->shadow_data) {
 int r;
 
 mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr,
@@ -380,7 +380,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
 return;
 
 fail_map:
-if (v->shadow_data) {
+if (v->shared->shadow_data) {
 vhost_iova_tree_remove(v->shared->iova_tree, mem_region);
 }
 
@@ -435,7 +435,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener 
*listener,
 
 llsize = int128_sub(llend, int128_make64(iova));
 
-if (v->shadow_data) {
+if (v->shared->shadow_data) {
 const DMAMap *result;
 const void *vaddr = memory_region_get_ram_ptr(section->mr) +
 section->offset_within_region +
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 7be2c30ad3..bf8e8327da 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -290,15 +290,6 @@ static ssize_t vhost_vdpa_receive(NetClientState *nc, 
const uint8_t *buf,
 return size;
 }
 
-/** From any vdpa net client, get the netclient of the first queue pair */
-static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
-{
-NICState *nic = qemu_get_nic(s->nc.peer);
-NetClientState *nc0 = qemu_get_peer(nic->ncs, 0);
-
-return DO_UPCAST(VhostVDPAState, nc, nc0);
-}
-
 static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
 {
 struct vhost_vdpa *v = &s->vhost_vdpa;
@@ -369,13 +360,12 @@ static int vhost_vdpa_net_data_start(NetClientState *nc)
 if (s->always_svq ||
 migration_is_setup_or_active(migrate_get_current()->state)) {
 v->shadow_vqs_enabled = true;
-v->shadow_data = true;
 } else {
 v->shadow_vqs_enabled = false;
-v->shadow_data = false;
 }
 
 if (v->index == 0) {
+v->shared->shadow_data = v->shadow_vqs_enabled;
 vhost_vdpa_net_data_start_first(s);
 return 0;
 }
@@ -523,7 +513,7 @@ dma_map_err:
 
 static int vhost_vdpa_net_cvq_start(NetClientState *nc)
 {
-VhostVDPAState *s, *s0;
+VhostVDPAState *s;
 struct vhost_vdpa *v;
 int64_t cvq_group;
 int r;
@@ -534,12 +524,10 @@ static int vhost_vdpa_net_cvq_start(NetClientState *nc)
 s = DO_UPCAST(VhostVDPAState, nc, nc);
 v = &s->vhost_vdpa;
 
-s0 = vhost_vdpa_net_first_nc_vdpa(s);
-v->shadow_data = s0->vhost_vdpa.shadow_vqs_enabled;
-v->shadow_vqs_enabled = s0->vhost_vdpa.shadow_vqs_enabled;
+v->shadow_vqs_enabled = v->shared->shadow_data;
 s->vhost_vdpa.address_space_id = VHOST_VDPA_GUEST_PA_ASID;
 
-if (s->vhost_vdpa.shadow_data) {
+if (v->shared->shadow_data) {
 /* SVQ is already configured for all virtqueues */
 goto out;
 }
@@ -1688,12 +1676,12 @@ static NetClientState 
*net_vhost_vdpa_init(NetClientState *peer,

[PULL 21/21] vdpa: move memory listener to vhost_vdpa_shared

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next patches will register the vhost_vdpa memory listener while the VM
is migrating at the destination, so we can map the memory to the device
before stopping the VM at the source.  The main goal is to reduce the
downtime.

However, the destination QEMU is unaware of which vhost_vdpa device will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the CVQ device.  Otherwise, it  will be the first one.

Move the memory listener to a common place rather than always in the
first / last vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-14-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h |  2 +-
 hw/virtio/vhost-vdpa.c | 84 --
 2 files changed, 40 insertions(+), 46 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 2abee2164a..8f54e5edd4 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -33,6 +33,7 @@ typedef struct VhostVDPAHostNotifier {
 /* Info shared by all vhost_vdpa device models */
 typedef struct vhost_vdpa_shared {
 int device_fd;
+MemoryListener listener;
 struct vhost_vdpa_iova_range iova_range;
 QLIST_HEAD(, vdpa_iommu) iommu_list;
 
@@ -51,7 +52,6 @@ typedef struct vhost_vdpa_shared {
 typedef struct vhost_vdpa {
 int index;
 uint32_t address_space_id;
-MemoryListener listener;
 uint64_t acked_features;
 bool shadow_vqs_enabled;
 /* Device suspended successfully */
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index e6276fb1e4..ddae494ca8 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -170,28 +170,28 @@ static void 
vhost_vdpa_iotlb_batch_begin_once(VhostVDPAShared *s)
 
 static void vhost_vdpa_listener_commit(MemoryListener *listener)
 {
-struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
+VhostVDPAShared *s = container_of(listener, VhostVDPAShared, listener);
 struct vhost_msg_v2 msg = {};
-int fd = v->shared->device_fd;
+int fd = s->device_fd;
 
-if (!(v->shared->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) {
+if (!(s->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) {
 return;
 }
 
-if (!v->shared->iotlb_batch_begin_sent) {
+if (!s->iotlb_batch_begin_sent) {
 return;
 }
 
 msg.type = VHOST_IOTLB_MSG_V2;
 msg.iotlb.type = VHOST_IOTLB_BATCH_END;
 
-trace_vhost_vdpa_listener_commit(v->shared, fd, msg.type, msg.iotlb.type);
+trace_vhost_vdpa_listener_commit(s, fd, msg.type, msg.iotlb.type);
 if (write(fd, &msg, sizeof(msg)) != sizeof(msg)) {
 error_report("failed to write, fd=%d, errno=%d (%s)",
  fd, errno, strerror(errno));
 }
 
-v->shared->iotlb_batch_begin_sent = false;
+s->iotlb_batch_begin_sent = false;
 }
 
 static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
@@ -246,7 +246,7 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
 static void vhost_vdpa_iommu_region_add(MemoryListener *listener,
 MemoryRegionSection *section)
 {
-struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
+VhostVDPAShared *s = container_of(listener, VhostVDPAShared, listener);
 
 struct vdpa_iommu *iommu;
 Int128 end;
@@ -270,7 +270,7 @@ static void vhost_vdpa_iommu_region_add(MemoryListener 
*listener,
 iommu_idx);
 iommu->iommu_offset = section->offset_within_address_space -
   section->offset_within_region;
-iommu->dev_shared = v->shared;
+iommu->dev_shared = s;
 
 ret = memory_region_register_iommu_notifier(section->mr, &iommu->n, NULL);
 if (ret) {
@@ -278,7 +278,7 @@ static void vhost_vdpa_iommu_region_add(MemoryListener 
*listener,
 return;
 }
 
-QLIST_INSERT_HEAD(&v->shared->iommu_list, iommu, iommu_next);
+QLIST_INSERT_HEAD(&s->iommu_list, iommu, iommu_next);
 memory_region_iommu_replay(iommu->iommu_mr, &iommu->n);
 
 return;
@@ -287,11 +287,11 @@ static void vhost_vdpa_iommu_region_add(MemoryListener 
*listener,
 static void vhost_vdpa_iommu_region_del(MemoryListener *listener,
 MemoryRegionSection *section)
 {
-struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
+VhostVDPAShared *s = container_of(listener, VhostVDPAShared, listener);
 
 struct vdpa_iommu *iommu;
 
-QLIST_FOREACH(iommu, &v->shared->iommu_list, iommu_next)
+QLIST_FOREACH(iommu, &s->iommu_list, iommu_next)
 {
 if (MEMORY_REGION(iommu->iommu_mr) == section->mr &&
 iommu->n.start == section->offset_within_region) {
@@ -307,7 +307,7 @@ static void vhost_vdpa_listen

[PULL 20/21] vdpa: use dev_shared in vdpa_iommu

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

The memory listener functions can call these too.  Make vdpa_iommu work
with VhostVDPAShared.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-13-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h |  2 +-
 hw/virtio/vhost-vdpa.c | 16 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 705c754776..2abee2164a 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -75,7 +75,7 @@ int vhost_vdpa_dma_unmap(VhostVDPAShared *s, uint32_t asid, 
hwaddr iova,
  hwaddr size);
 
 typedef struct vdpa_iommu {
-struct vhost_vdpa *dev;
+VhostVDPAShared *dev_shared;
 IOMMUMemoryRegion *iommu_mr;
 hwaddr iommu_offset;
 IOMMUNotifier n;
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index ada3cc5d7c..e6276fb1e4 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -199,7 +199,7 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
 struct vdpa_iommu *iommu = container_of(n, struct vdpa_iommu, n);
 
 hwaddr iova = iotlb->iova + iommu->iommu_offset;
-struct vhost_vdpa *v = iommu->dev;
+VhostVDPAShared *s = iommu->dev_shared;
 void *vaddr;
 int ret;
 Int128 llend;
@@ -212,10 +212,10 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
 RCU_READ_LOCK_GUARD();
 /* check if RAM section out of device range */
 llend = int128_add(int128_makes64(iotlb->addr_mask), int128_makes64(iova));
-if (int128_gt(llend, int128_make64(v->shared->iova_range.last))) {
+if (int128_gt(llend, int128_make64(s->iova_range.last))) {
 error_report("RAM section out of device range (max=0x%" PRIx64
  ", end addr=0x%" PRIx64 ")",
- v->shared->iova_range.last, int128_get64(llend));
+ s->iova_range.last, int128_get64(llend));
 return;
 }
 
@@ -225,20 +225,20 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
 if (!memory_get_xlat_addr(iotlb, &vaddr, NULL, &read_only, NULL)) {
 return;
 }
-ret = vhost_vdpa_dma_map(v->shared, VHOST_VDPA_GUEST_PA_ASID, iova,
+ret = vhost_vdpa_dma_map(s, VHOST_VDPA_GUEST_PA_ASID, iova,
  iotlb->addr_mask + 1, vaddr, read_only);
 if (ret) {
 error_report("vhost_vdpa_dma_map(%p, 0x%" HWADDR_PRIx ", "
  "0x%" HWADDR_PRIx ", %p) = %d (%m)",
- v, iova, iotlb->addr_mask + 1, vaddr, ret);
+ s, iova, iotlb->addr_mask + 1, vaddr, ret);
 }
 } else {
-ret = vhost_vdpa_dma_unmap(v->shared, VHOST_VDPA_GUEST_PA_ASID, iova,
+ret = vhost_vdpa_dma_unmap(s, VHOST_VDPA_GUEST_PA_ASID, iova,
iotlb->addr_mask + 1);
 if (ret) {
 error_report("vhost_vdpa_dma_unmap(%p, 0x%" HWADDR_PRIx ", "
  "0x%" HWADDR_PRIx ") = %d (%m)",
- v, iova, iotlb->addr_mask + 1, ret);
+ s, iova, iotlb->addr_mask + 1, ret);
 }
 }
 }
@@ -270,7 +270,7 @@ static void vhost_vdpa_iommu_region_add(MemoryListener 
*listener,
 iommu_idx);
 iommu->iommu_offset = section->offset_within_address_space -
   section->offset_within_region;
-iommu->dev = v;
+iommu->dev_shared = v->shared;
 
 ret = memory_region_register_iommu_notifier(section->mr, &iommu->n, NULL);
 if (ret) {
-- 
MST




[PULL 08/21] vdpa: do not set virtio status bits if unneeded

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next commits will set DRIVER and ACKNOWLEDGE flags repeatedly in the
case of a migration destination.  Let's save ioctls with this.

Signed-off-by: Eugenio Pérez 
Message-Id: <20231215172830.2540987-2-epere...@redhat.com>
---
 hw/virtio/vhost-vdpa.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 819b2d811a..401dfa96fd 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -511,6 +511,10 @@ static int vhost_vdpa_add_status(struct vhost_dev *dev, 
uint8_t status)
 if (ret < 0) {
 return ret;
 }
+if ((s & status) == status) {
+/* Don't set bits already set */
+return 0;
+}
 
 s |= status;
 
-- 
MST




[PULL 17/21] vdpa: remove msg type of vhost_vdpa

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

It is always VHOST_IOTLB_MSG_V2. We can always make it back per
vhost_dev if needed.

This change makes easier for vhost_vdpa_map and unmap not to depend on
vhost_vdpa but only in VhostVDPAShared.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-10-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h | 1 -
 hw/virtio/vhost-vdpa.c | 9 -
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 11ac14085a..5bd964dac5 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -49,7 +49,6 @@ typedef struct vhost_vdpa_shared {
 
 typedef struct vhost_vdpa {
 int index;
-uint32_t msg_type;
 uint32_t address_space_id;
 MemoryListener listener;
 uint64_t acked_features;
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 99597c3179..cbfcf18883 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -93,7 +93,7 @@ int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t asid, 
hwaddr iova,
 int fd = v->shared->device_fd;
 int ret = 0;
 
-msg.type = v->msg_type;
+msg.type = VHOST_IOTLB_MSG_V2;
 msg.asid = asid;
 msg.iotlb.iova = iova;
 msg.iotlb.size = size;
@@ -125,7 +125,7 @@ int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t 
asid, hwaddr iova,
 int fd = v->shared->device_fd;
 int ret = 0;
 
-msg.type = v->msg_type;
+msg.type = VHOST_IOTLB_MSG_V2;
 msg.asid = asid;
 msg.iotlb.iova = iova;
 msg.iotlb.size = size;
@@ -147,7 +147,7 @@ static void vhost_vdpa_listener_begin_batch(struct 
vhost_vdpa *v)
 {
 int fd = v->shared->device_fd;
 struct vhost_msg_v2 msg = {
-.type = v->msg_type,
+.type = VHOST_IOTLB_MSG_V2,
 .iotlb.type = VHOST_IOTLB_BATCH_BEGIN,
 };
 
@@ -183,7 +183,7 @@ static void vhost_vdpa_listener_commit(MemoryListener 
*listener)
 return;
 }
 
-msg.type = v->msg_type;
+msg.type = VHOST_IOTLB_MSG_V2;
 msg.iotlb.type = VHOST_IOTLB_BATCH_END;
 
 trace_vhost_vdpa_listener_commit(v->shared, fd, msg.type, msg.iotlb.type);
@@ -597,7 +597,6 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void 
*opaque, Error **errp)
 v->dev = dev;
 dev->opaque =  opaque ;
 v->listener = vhost_vdpa_memory_listener;
-v->msg_type = VHOST_IOTLB_MSG_V2;
 vhost_vdpa_init_svq(dev, v);
 
 error_propagate(&dev->migration_blocker, v->migration_blocker);
-- 
MST




[PULL 16/21] vdpa: move backend_cap to vhost_vdpa_shared

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next patches will register the vhost_vdpa memory listener while the VM
is migrating at the destination, so we can map the memory to the device
before stopping the VM at the source.  The main goal is to reduce the
downtime.

However, the destination QEMU is unaware of which vhost_vdpa device will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the CVQ device.  Otherwise, it  will be the first one.

Move the backend_cap member to VhostVDPAShared so all vhost_vdpa can use
it, rather than always in the first / last vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-9-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h | 3 +++
 hw/virtio/vhost-vdpa.c | 8 +---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 05219bbcf7..11ac14085a 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -38,6 +38,9 @@ typedef struct vhost_vdpa_shared {
 /* IOVA mapping used by the Shadow Virtqueue */
 VhostIOVATree *iova_tree;
 
+/* Copy of backend features */
+uint64_t backend_cap;
+
 bool iotlb_batch_begin_sent;
 
 /* Vdpa must send shadow addresses as IOTLB key for data queues, not GPA */
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 2ecaedb686..99597c3179 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -161,7 +161,7 @@ static void vhost_vdpa_listener_begin_batch(struct 
vhost_vdpa *v)
 
 static void vhost_vdpa_iotlb_batch_begin_once(struct vhost_vdpa *v)
 {
-if (v->dev->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH) &&
+if (v->shared->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH) &&
 !v->shared->iotlb_batch_begin_sent) {
 vhost_vdpa_listener_begin_batch(v);
 }
@@ -172,11 +172,10 @@ static void vhost_vdpa_iotlb_batch_begin_once(struct 
vhost_vdpa *v)
 static void vhost_vdpa_listener_commit(MemoryListener *listener)
 {
 struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
-struct vhost_dev *dev = v->dev;
 struct vhost_msg_v2 msg = {};
 int fd = v->shared->device_fd;
 
-if (!(dev->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) {
+if (!(v->shared->backend_cap & (0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH))) {
 return;
 }
 
@@ -838,6 +837,8 @@ static int vhost_vdpa_set_features(struct vhost_dev *dev,
 
 static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev)
 {
+struct vhost_vdpa *v = dev->opaque;
+
 uint64_t features;
 uint64_t f = 0x1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2 |
 0x1ULL << VHOST_BACKEND_F_IOTLB_BATCH |
@@ -859,6 +860,7 @@ static int vhost_vdpa_set_backend_cap(struct vhost_dev *dev)
 }
 
 dev->backend_cap = features;
+v->shared->backend_cap = features;
 
 return 0;
 }
-- 
MST




[PULL 13/21] vdpa: use vdpa shared for tracing

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

By the end of this series dma_map and dma_unmap functions don't have the
vdpa device for tracing.  Movinge trace function to shared member one.
Print it also in the vdpa initialization so log reader can relate them.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-6-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/vhost-vdpa.c | 26 ++
 hw/virtio/trace-events | 14 +++---
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 97588848fc..720cffbc08 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -101,7 +101,7 @@ int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t asid, 
hwaddr iova,
 msg.iotlb.perm = readonly ? VHOST_ACCESS_RO : VHOST_ACCESS_RW;
 msg.iotlb.type = VHOST_IOTLB_UPDATE;
 
-trace_vhost_vdpa_dma_map(v, fd, msg.type, msg.asid, msg.iotlb.iova,
+trace_vhost_vdpa_dma_map(v->shared, fd, msg.type, msg.asid, msg.iotlb.iova,
  msg.iotlb.size, msg.iotlb.uaddr, msg.iotlb.perm,
  msg.iotlb.type);
 
@@ -131,8 +131,8 @@ int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t 
asid, hwaddr iova,
 msg.iotlb.size = size;
 msg.iotlb.type = VHOST_IOTLB_INVALIDATE;
 
-trace_vhost_vdpa_dma_unmap(v, fd, msg.type, msg.asid, msg.iotlb.iova,
-   msg.iotlb.size, msg.iotlb.type);
+trace_vhost_vdpa_dma_unmap(v->shared, fd, msg.type, msg.asid,
+   msg.iotlb.iova, msg.iotlb.size, msg.iotlb.type);
 
 if (write(fd, &msg, sizeof(msg)) != sizeof(msg)) {
 error_report("failed to write, fd=%d, errno=%d (%s)",
@@ -151,7 +151,8 @@ static void vhost_vdpa_listener_begin_batch(struct 
vhost_vdpa *v)
 .iotlb.type = VHOST_IOTLB_BATCH_BEGIN,
 };
 
-trace_vhost_vdpa_listener_begin_batch(v, fd, msg.type, msg.iotlb.type);
+trace_vhost_vdpa_listener_begin_batch(v->shared, fd, msg.type,
+  msg.iotlb.type);
 if (write(fd, &msg, sizeof(msg)) != sizeof(msg)) {
 error_report("failed to write, fd=%d, errno=%d (%s)",
  fd, errno, strerror(errno));
@@ -186,7 +187,7 @@ static void vhost_vdpa_listener_commit(MemoryListener 
*listener)
 msg.type = v->msg_type;
 msg.iotlb.type = VHOST_IOTLB_BATCH_END;
 
-trace_vhost_vdpa_listener_commit(v, fd, msg.type, msg.iotlb.type);
+trace_vhost_vdpa_listener_commit(v->shared, fd, msg.type, msg.iotlb.type);
 if (write(fd, &msg, sizeof(msg)) != sizeof(msg)) {
 error_report("failed to write, fd=%d, errno=%d (%s)",
  fd, errno, strerror(errno));
@@ -329,7 +330,8 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
 
 if (unlikely((section->offset_within_address_space & ~page_mask) !=
  (section->offset_within_region & ~page_mask))) {
-trace_vhost_vdpa_listener_region_add_unaligned(v, section->mr->name,
+trace_vhost_vdpa_listener_region_add_unaligned(v->shared,
+   section->mr->name,
section->offset_within_address_space & ~page_mask,
section->offset_within_region & ~page_mask);
 return;
@@ -349,7 +351,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
 section->offset_within_region +
 (iova - section->offset_within_address_space);
 
-trace_vhost_vdpa_listener_region_add(v, iova, int128_get64(llend),
+trace_vhost_vdpa_listener_region_add(v->shared, iova, int128_get64(llend),
  vaddr, section->readonly);
 
 llsize = int128_sub(llend, int128_make64(iova));
@@ -417,7 +419,8 @@ static void vhost_vdpa_listener_region_del(MemoryListener 
*listener,
 
 if (unlikely((section->offset_within_address_space & ~page_mask) !=
  (section->offset_within_region & ~page_mask))) {
-trace_vhost_vdpa_listener_region_del_unaligned(v, section->mr->name,
+trace_vhost_vdpa_listener_region_del_unaligned(v->shared,
+   section->mr->name,
section->offset_within_address_space & ~page_mask,
section->offset_within_region & ~page_mask);
 return;
@@ -426,7 +429,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener 
*listener,
 iova = ROUND_UP(section->offset_within_address_space, page_size);
 llend = vhost_vdpa_section_end(section, page_mask);
 
-trace_vhost_vdpa_listener_region_del(v, iova,
+trace_vhost_vdpa_listener_region_del(v->shared, iova,
 int128_get64(int128_sub(llend, int128_one(;
 
 if (int128_ge(int128_make64(iova), llend)) {
@@ -587,12 +590,11 @@ static void vhost_vdpa_init_svq(struct vhost_dev *hdev, 
struct vhost_vdpa *v)

[PULL 04/21] vhost-scsi: Add support for a worker thread per virtqueue

2023-12-26 Thread Michael S. Tsirkin
From: Mike Christie 

This adds support for vhost-scsi to be able to create a worker thread
per virtqueue. Right now for vhost-net we get a worker thread per
tx/rx virtqueue pair which scales nicely as we add more virtqueues and
CPUs, but for scsi we get the single worker thread that's shared by all
virtqueues. When trying to send IO to more than 2 virtqueues the single
thread becomes a bottlneck.

This patch adds a new setting, worker_per_virtqueue, which can be set
to:

false: Existing behavior where we get the single worker thread.
true: Create a worker per IO virtqueue.

Signed-off-by: Mike Christie 
Reviewed-by: Stefan Hajnoczi 

Message-Id: <20231204231618.21962-3-michael.chris...@oracle.com>
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Stefano Garzarella 
---
 include/hw/virtio/virtio-scsi.h |  1 +
 hw/scsi/vhost-scsi.c| 62 +
 2 files changed, 63 insertions(+)

diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 7f0573b1bf..7be0105918 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -51,6 +51,7 @@ typedef struct virtio_scsi_config VirtIOSCSIConfig;
 struct VirtIOSCSIConf {
 uint32_t num_queues;
 uint32_t virtqueue_size;
+bool worker_per_virtqueue;
 bool seg_max_adjust;
 uint32_t max_sectors;
 uint32_t cmd_per_lun;
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 3126df9e1d..08aa7534df 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -165,6 +165,59 @@ static const VMStateDescription vmstate_virtio_vhost_scsi 
= {
 .pre_save = vhost_scsi_pre_save,
 };
 
+static int vhost_scsi_set_workers(VHostSCSICommon *vsc, bool per_virtqueue)
+{
+struct vhost_dev *dev = &vsc->dev;
+struct vhost_vring_worker vq_worker;
+struct vhost_worker_state worker;
+int i, ret;
+
+/* Use default worker */
+if (!per_virtqueue || dev->nvqs == VHOST_SCSI_VQ_NUM_FIXED + 1) {
+return 0;
+}
+
+/*
+ * ctl/evt share the first worker since it will be rare for them
+ * to send cmds while IO is running.
+ */
+for (i = VHOST_SCSI_VQ_NUM_FIXED + 1; i < dev->nvqs; i++) {
+memset(&worker, 0, sizeof(worker));
+
+ret = dev->vhost_ops->vhost_new_worker(dev, &worker);
+if (ret == -ENOTTY) {
+/*
+ * worker ioctls are not implemented so just ignore and
+ * and continue device setup.
+ */
+warn_report("vhost-scsi: Backend supports a single worker. "
+"Ignoring worker_per_virtqueue=true setting.");
+ret = 0;
+break;
+} else if (ret) {
+break;
+}
+
+memset(&vq_worker, 0, sizeof(vq_worker));
+vq_worker.worker_id = worker.worker_id;
+vq_worker.index = i;
+
+ret = dev->vhost_ops->vhost_attach_vring_worker(dev, &vq_worker);
+if (ret == -ENOTTY) {
+/*
+ * It's a bug for the kernel to have supported the worker creation
+ * ioctl but not attach.
+ */
+dev->vhost_ops->vhost_free_worker(dev, &worker);
+break;
+} else if (ret) {
+break;
+}
+}
+
+return ret;
+}
+
 static void vhost_scsi_realize(DeviceState *dev, Error **errp)
 {
 VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
@@ -232,6 +285,13 @@ static void vhost_scsi_realize(DeviceState *dev, Error 
**errp)
 goto free_vqs;
 }
 
+ret = vhost_scsi_set_workers(vsc, vs->conf.worker_per_virtqueue);
+if (ret < 0) {
+error_setg(errp, "vhost-scsi: vhost worker setup failed: %s",
+   strerror(-ret));
+goto free_vqs;
+}
+
 /* At present, channel and lun both are 0 for bootable vhost-scsi disk */
 vsc->channel = 0;
 vsc->lun = 0;
@@ -297,6 +357,8 @@ static Property vhost_scsi_properties[] = {
  VIRTIO_SCSI_F_T10_PI,
  false),
 DEFINE_PROP_BOOL("migratable", VHostSCSICommon, migratable, false),
+DEFINE_PROP_BOOL("worker_per_virtqueue", VirtIOSCSICommon,
+ conf.worker_per_virtqueue, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
MST




[PULL 18/21] vdpa: move iommu_list to vhost_vdpa_shared

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next patches will register the vhost_vdpa memory listener while the VM
is migrating at the destination, so we can map the memory to the device
before stopping the VM at the source.  The main goal is to reduce the
downtime.

However, the destination QEMU is unaware of which vhost_vdpa device will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the CVQ device.  Otherwise, it  will be the first one.

Move the iommu_list member to VhostVDPAShared so all vhost_vdpa can use
it, rather than always in the first / last vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-11-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h | 2 +-
 hw/virtio/vhost-vdpa.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 5bd964dac5..3880b9e7f2 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -34,6 +34,7 @@ typedef struct VhostVDPAHostNotifier {
 typedef struct vhost_vdpa_shared {
 int device_fd;
 struct vhost_vdpa_iova_range iova_range;
+QLIST_HEAD(, vdpa_iommu) iommu_list;
 
 /* IOVA mapping used by the Shadow Virtqueue */
 VhostIOVATree *iova_tree;
@@ -62,7 +63,6 @@ typedef struct vhost_vdpa {
 struct vhost_dev *dev;
 Error *migration_blocker;
 VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
-QLIST_HEAD(, vdpa_iommu) iommu_list;
 IOMMUNotifier n;
 } VhostVDPA;
 
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index cbfcf18883..a2713b9f0b 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -279,7 +279,7 @@ static void vhost_vdpa_iommu_region_add(MemoryListener 
*listener,
 return;
 }
 
-QLIST_INSERT_HEAD(&v->iommu_list, iommu, iommu_next);
+QLIST_INSERT_HEAD(&v->shared->iommu_list, iommu, iommu_next);
 memory_region_iommu_replay(iommu->iommu_mr, &iommu->n);
 
 return;
@@ -292,7 +292,7 @@ static void vhost_vdpa_iommu_region_del(MemoryListener 
*listener,
 
 struct vdpa_iommu *iommu;
 
-QLIST_FOREACH(iommu, &v->iommu_list, iommu_next)
+QLIST_FOREACH(iommu, &v->shared->iommu_list, iommu_next)
 {
 if (MEMORY_REGION(iommu->iommu_mr) == section->mr &&
 iommu->n.start == section->offset_within_region) {
-- 
MST




[PULL 10/21] vdpa: move iova tree to the shared struct

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next patches will register the vhost_vdpa memory listener while the VM
is migrating at the destination, so we can map the memory to the device
before stopping the VM at the source.  The main goal is to reduce the
downtime.

However, the destination QEMU is unaware of which vhost_vdpa device will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the CVQ device.  Otherwise, it  will be the first one.

Move the iova tree to VhostVDPAShared so all vhost_vdpa can use it,
rather than always in the first or last vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-3-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h |  4 +--
 hw/virtio/vhost-vdpa.c | 19 ++--
 net/vhost-vdpa.c   | 54 +++---
 3 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index eb1a56d75a..ac036055d3 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -32,6 +32,8 @@ typedef struct VhostVDPAHostNotifier {
 
 /* Info shared by all vhost_vdpa device models */
 typedef struct vhost_vdpa_shared {
+/* IOVA mapping used by the Shadow Virtqueue */
+VhostIOVATree *iova_tree;
 } VhostVDPAShared;
 
 typedef struct vhost_vdpa {
@@ -48,8 +50,6 @@ typedef struct vhost_vdpa {
 bool shadow_data;
 /* Device suspended successfully */
 bool suspended;
-/* IOVA mapping used by the Shadow Virtqueue */
-VhostIOVATree *iova_tree;
 VhostVDPAShared *shared;
 GPtrArray *shadow_vqs;
 const VhostShadowVirtqueueOps *shadow_vq_ops;
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 401dfa96fd..95a179a082 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -358,7 +358,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
 mem_region.size = int128_get64(llsize) - 1,
 mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly),
 
-r = vhost_iova_tree_map_alloc(v->iova_tree, &mem_region);
+r = vhost_iova_tree_map_alloc(v->shared->iova_tree, &mem_region);
 if (unlikely(r != IOVA_OK)) {
 error_report("Can't allocate a mapping (%d)", r);
 goto fail;
@@ -379,7 +379,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
 
 fail_map:
 if (v->shadow_data) {
-vhost_iova_tree_remove(v->iova_tree, mem_region);
+vhost_iova_tree_remove(v->shared->iova_tree, mem_region);
 }
 
 fail:
@@ -441,13 +441,13 @@ static void vhost_vdpa_listener_region_del(MemoryListener 
*listener,
 .size = int128_get64(llsize) - 1,
 };
 
-result = vhost_iova_tree_find_iova(v->iova_tree, &mem_region);
+result = vhost_iova_tree_find_iova(v->shared->iova_tree, &mem_region);
 if (!result) {
 /* The memory listener map wasn't mapped */
 return;
 }
 iova = result->iova;
-vhost_iova_tree_remove(v->iova_tree, *result);
+vhost_iova_tree_remove(v->shared->iova_tree, *result);
 }
 vhost_vdpa_iotlb_batch_begin_once(v);
 /*
@@ -1063,7 +1063,8 @@ static void vhost_vdpa_svq_unmap_ring(struct vhost_vdpa 
*v, hwaddr addr)
 const DMAMap needle = {
 .translated_addr = addr,
 };
-const DMAMap *result = vhost_iova_tree_find_iova(v->iova_tree, &needle);
+const DMAMap *result = vhost_iova_tree_find_iova(v->shared->iova_tree,
+ &needle);
 hwaddr size;
 int r;
 
@@ -1079,7 +1080,7 @@ static void vhost_vdpa_svq_unmap_ring(struct vhost_vdpa 
*v, hwaddr addr)
 return;
 }
 
-vhost_iova_tree_remove(v->iova_tree, *result);
+vhost_iova_tree_remove(v->shared->iova_tree, *result);
 }
 
 static void vhost_vdpa_svq_unmap_rings(struct vhost_dev *dev,
@@ -1107,7 +1108,7 @@ static bool vhost_vdpa_svq_map_ring(struct vhost_vdpa *v, 
DMAMap *needle,
 {
 int r;
 
-r = vhost_iova_tree_map_alloc(v->iova_tree, needle);
+r = vhost_iova_tree_map_alloc(v->shared->iova_tree, needle);
 if (unlikely(r != IOVA_OK)) {
 error_setg(errp, "Cannot allocate iova (%d)", r);
 return false;
@@ -1119,7 +1120,7 @@ static bool vhost_vdpa_svq_map_ring(struct vhost_vdpa *v, 
DMAMap *needle,
needle->perm == IOMMU_RO);
 if (unlikely(r != 0)) {
 error_setg_errno(errp, -r, "Cannot map region to device");
-vhost_iova_tree_remove(v->iova_tree, *needle);
+vhost_iova_tree_remove(v->shared->iova_tree, *needle);
 }
 
 return r == 0;
@@ -1220,7 +1221,7 @@ static bool vhost_vdpa_svqs_start(struct vhost_dev *dev)
 goto err;
 }
 
-vhost_svq_start(svq, dev->vdev, vq, v->iova_tree);
+vhost_svq_start(svq

[PULL 11/21] vdpa: move iova_range to vhost_vdpa_shared

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

Next patches will register the vhost_vdpa memory listener while the VM
is migrating at the destination, so we can map the memory to the device
before stopping the VM at the source.  The main goal is to reduce the
downtime.

However, the destination QEMU is unaware of which vhost_vdpa device will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the CVQ device.  Otherwise, it  will be the first one.

Move the iova range to VhostVDPAShared so all vhost_vdpa can use it,
rather than always in the first or last vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-4-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h |  3 ++-
 hw/virtio/vdpa-dev.c   |  5 -
 hw/virtio/vhost-vdpa.c | 16 ++--
 net/vhost-vdpa.c   | 10 +-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index ac036055d3..8d52a7e498 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -32,6 +32,8 @@ typedef struct VhostVDPAHostNotifier {
 
 /* Info shared by all vhost_vdpa device models */
 typedef struct vhost_vdpa_shared {
+struct vhost_vdpa_iova_range iova_range;
+
 /* IOVA mapping used by the Shadow Virtqueue */
 VhostIOVATree *iova_tree;
 } VhostVDPAShared;
@@ -43,7 +45,6 @@ typedef struct vhost_vdpa {
 bool iotlb_batch_begin_sent;
 uint32_t address_space_id;
 MemoryListener listener;
-struct vhost_vdpa_iova_range iova_range;
 uint64_t acked_features;
 bool shadow_vqs_enabled;
 /* Vdpa must send shadow addresses as IOTLB key for data queues, not GPA */
diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
index f22d5d5bc0..457960d28a 100644
--- a/hw/virtio/vdpa-dev.c
+++ b/hw/virtio/vdpa-dev.c
@@ -114,7 +114,8 @@ static void vhost_vdpa_device_realize(DeviceState *dev, 
Error **errp)
strerror(-ret));
 goto free_vqs;
 }
-v->vdpa.iova_range = iova_range;
+v->vdpa.shared = g_new0(VhostVDPAShared, 1);
+v->vdpa.shared->iova_range = iova_range;
 
 ret = vhost_dev_init(&v->dev, &v->vdpa, VHOST_BACKEND_TYPE_VDPA, 0, NULL);
 if (ret < 0) {
@@ -162,6 +163,7 @@ vhost_cleanup:
 vhost_dev_cleanup(&v->dev);
 free_vqs:
 g_free(vqs);
+g_free(v->vdpa.shared);
 out:
 qemu_close(v->vhostfd);
 v->vhostfd = -1;
@@ -184,6 +186,7 @@ static void vhost_vdpa_device_unrealize(DeviceState *dev)
 g_free(s->config);
 g_free(s->dev.vqs);
 vhost_dev_cleanup(&s->dev);
+g_free(s->vdpa.shared);
 qemu_close(s->vhostfd);
 s->vhostfd = -1;
 }
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 95a179a082..5ff1d43ba9 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -213,10 +213,10 @@ static void vhost_vdpa_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
 RCU_READ_LOCK_GUARD();
 /* check if RAM section out of device range */
 llend = int128_add(int128_makes64(iotlb->addr_mask), int128_makes64(iova));
-if (int128_gt(llend, int128_make64(v->iova_range.last))) {
+if (int128_gt(llend, int128_make64(v->shared->iova_range.last))) {
 error_report("RAM section out of device range (max=0x%" PRIx64
  ", end addr=0x%" PRIx64 ")",
- v->iova_range.last, int128_get64(llend));
+ v->shared->iova_range.last, int128_get64(llend));
 return;
 }
 
@@ -316,8 +316,10 @@ static void vhost_vdpa_listener_region_add(MemoryListener 
*listener,
 int page_size = qemu_target_page_size();
 int page_mask = -page_size;
 
-if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
-v->iova_range.last, page_mask)) {
+if (vhost_vdpa_listener_skipped_section(section,
+v->shared->iova_range.first,
+v->shared->iova_range.last,
+page_mask)) {
 return;
 }
 if (memory_region_is_iommu(section->mr)) {
@@ -403,8 +405,10 @@ static void vhost_vdpa_listener_region_del(MemoryListener 
*listener,
 int page_size = qemu_target_page_size();
 int page_mask = -page_size;
 
-if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
-v->iova_range.last, page_mask)) {
+if (vhost_vdpa_listener_skipped_section(section,
+v->shared->iova_range.first,
+v->shared->iova_range.last,
+page_mask)) {
 return;
 }
 if (memory_region_is_iommu(section->mr)) {
diff --git a/net/vhost-vdpa.c b/net/vhost-

[PULL 01/21] virtio: rng: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

2023-12-26 Thread Michael S. Tsirkin
From: Mathieu Poirier 

Since the driver doesn't support interrupts, we must return early when
index is set to VIRTIO_CONFIG_IRQ_IDX.  Basically the same thing Viresh
did for "91208dd297f2 virtio: i2c: Check notifier helpers for
VIRTIO_CONFIG_IRQ_IDX".

Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX")
Signed-off-by: Mathieu Poirier 
Message-Id: <20231025171841.3379663-1-mathieu.poir...@linaro.org>
Tested-by: Leo Yan 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/vhost-user-rng.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/hw/virtio/vhost-user-rng.c b/hw/virtio/vhost-user-rng.c
index efc54cd3fb..24ac1a22c8 100644
--- a/hw/virtio/vhost-user-rng.c
+++ b/hw/virtio/vhost-user-rng.c
@@ -129,6 +129,14 @@ static void vu_rng_guest_notifier_mask(VirtIODevice *vdev, 
int idx, bool mask)
 {
 VHostUserRNG *rng = VHOST_USER_RNG(vdev);
 
+/*
+ * We don't support interrupts, return early if index is set to
+ * VIRTIO_CONFIG_IRQ_IDX.
+ */
+if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+return;
+}
+
 vhost_virtqueue_mask(&rng->vhost_dev, vdev, idx, mask);
 }
 
@@ -136,6 +144,14 @@ static bool vu_rng_guest_notifier_pending(VirtIODevice 
*vdev, int idx)
 {
 VHostUserRNG *rng = VHOST_USER_RNG(vdev);
 
+/*
+ * We don't support interrupts, return early if index is set to
+ * VIRTIO_CONFIG_IRQ_IDX.
+ */
+if (idx == VIRTIO_CONFIG_IRQ_IDX) {
+return false;
+}
+
 return vhost_virtqueue_pending(&rng->vhost_dev, idx);
 }
 
-- 
MST




[PULL 05/21] hw/acpi: propagate vcpu hotplug after switch to modern interface

2023-12-26 Thread Michael S. Tsirkin
From: Aaron Young 

If a vcpu with an apic-id that is not supported by the legacy
interface (>255) is hot-plugged, the legacy code will dynamically switch
to the modern interface. However, the hotplug event is not forwarded to
the new interface resulting in the vcpu not being fully/properly added
to the machine config. This BUG is evidenced by OVMF when it
it attempts to count the vcpus and reports an inconsistent vcpu count
reported by the fw_cfg interface and the modern hotpug interface.

Fix is to propagate the hotplug event after making the switch from
the legacy interface to the modern interface.

Cc: "Michael S. Tsirkin" 
Cc: Igor Mammedov 
Signed-off-by: Aaron Young 
Message-Id: 
<0e8a9baebbb29f2a6c87fd08e43dc2ac4019759a.1702398644.git.aaron.yo...@oracle.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/acpi/cpu_hotplug.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 634bbecb31..6f78db0ccb 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -59,7 +59,8 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
 },
 };
 
-static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu)
+static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
+ bool *swtchd_to_modern)
 {
 CPUClass *k = CPU_GET_CLASS(cpu);
 int64_t cpu_id;
@@ -68,23 +69,34 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, 
CPUState *cpu)
 if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
 object_property_set_bool(g->device, "cpu-hotplug-legacy", false,
  &error_abort);
+*swtchd_to_modern = true;
 return;
 }
 
+*swtchd_to_modern = false;
 g->sts[cpu_id / 8] |= (1 << (cpu_id % 8));
 }
 
 void legacy_acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
  AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
 {
-acpi_set_cpu_present_bit(g, CPU(dev));
-acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
+bool swtchd_to_modern;
+Error *local_err = NULL;
+
+acpi_set_cpu_present_bit(g, CPU(dev), &swtchd_to_modern);
+if (swtchd_to_modern) {
+/* propagate the hotplug to the modern interface */
+hotplug_handler_plug(hotplug_dev, dev, &local_err);
+} else {
+acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
+}
 }
 
 void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
   AcpiCpuHotplug *gpe_cpu, uint16_t base)
 {
 CPUState *cpu;
+bool swtchd_to_modern;
 
 memory_region_init_io(&gpe_cpu->io, owner, &AcpiCpuHotplug_ops,
   gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
@@ -92,7 +104,7 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, 
Object *owner,
 gpe_cpu->device = owner;
 
 CPU_FOREACH(cpu) {
-acpi_set_cpu_present_bit(gpe_cpu, cpu);
+acpi_set_cpu_present_bit(gpe_cpu, cpu, &swtchd_to_modern);
 }
 }
 
-- 
MST




[PULL 09/21] vdpa: add VhostVDPAShared

2023-12-26 Thread Michael S. Tsirkin
From: Eugenio Pérez 

It will hold properties shared among all vhost_vdpa instances associated
with of the same device.  For example, we just need one iova_tree or one
memory listener for the entire device.

Next patches will register the vhost_vdpa memory listener at the
beginning of the VM migration at the destination. This enables QEMU to
map the memory to the device before stopping the VM at the source,
instead of doing while both source and destination are stopped, thus
minimizing the downtime.

However, the destination QEMU is unaware of which vhost_vdpa struct will
register its memory_listener.  If the source guest has CVQ enabled, it
will be the one associated with the CVQ.  Otherwise, it will be the
first one.

Save the memory operations related members in a common place rather than
always in the first / last vhost_vdpa.

Signed-off-by: Eugenio Pérez 
Acked-by: Jason Wang 
Message-Id: <20231221174322.3130442-2-epere...@redhat.com>
Tested-by: Lei Yang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-vdpa.h |  5 +
 net/vhost-vdpa.c   | 24 ++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 5407d54fd7..eb1a56d75a 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -30,6 +30,10 @@ typedef struct VhostVDPAHostNotifier {
 void *addr;
 } VhostVDPAHostNotifier;
 
+/* Info shared by all vhost_vdpa device models */
+typedef struct vhost_vdpa_shared {
+} VhostVDPAShared;
+
 typedef struct vhost_vdpa {
 int device_fd;
 int index;
@@ -46,6 +50,7 @@ typedef struct vhost_vdpa {
 bool suspended;
 /* IOVA mapping used by the Shadow Virtqueue */
 VhostIOVATree *iova_tree;
+VhostVDPAShared *shared;
 GPtrArray *shadow_vqs;
 const VhostShadowVirtqueueOps *shadow_vq_ops;
 void *shadow_vq_ops_opaque;
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index d0614d7954..8b661b9e6d 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -240,6 +240,10 @@ static void vhost_vdpa_cleanup(NetClientState *nc)
 qemu_close(s->vhost_vdpa.device_fd);
 s->vhost_vdpa.device_fd = -1;
 }
+if (s->vhost_vdpa.index != 0) {
+return;
+}
+g_free(s->vhost_vdpa.shared);
 }
 
 /** Dummy SetSteeringEBPF to support RSS for vhost-vdpa backend  */
@@ -1661,6 +1665,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState 
*peer,
bool svq,
struct vhost_vdpa_iova_range iova_range,
uint64_t features,
+   VhostVDPAShared *shared,
Error **errp)
 {
 NetClientState *nc = NULL;
@@ -1696,6 +1701,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState 
*peer,
 if (queue_pair_index == 0) {
 vhost_vdpa_net_valid_svq_features(features,
   &s->vhost_vdpa.migration_blocker);
+s->vhost_vdpa.shared = g_new0(VhostVDPAShared, 1);
 } else if (!is_datapath) {
 s->cvq_cmd_out_buffer = mmap(NULL, vhost_vdpa_net_cvq_cmd_page_len(),
  PROT_READ | PROT_WRITE,
@@ -1708,11 +1714,16 @@ static NetClientState 
*net_vhost_vdpa_init(NetClientState *peer,
 s->vhost_vdpa.shadow_vq_ops_opaque = s;
 s->cvq_isolated = cvq_isolated;
 }
+if (queue_pair_index != 0) {
+s->vhost_vdpa.shared = shared;
+}
+
 ret = vhost_vdpa_add(nc, (void *)&s->vhost_vdpa, queue_pair_index, nvqs);
 if (ret) {
 qemu_del_net_client(nc);
 return NULL;
 }
+
 return nc;
 }
 
@@ -1824,17 +1835,26 @@ int net_init_vhost_vdpa(const Netdev *netdev, const 
char *name,
 ncs = g_malloc0(sizeof(*ncs) * queue_pairs);
 
 for (i = 0; i < queue_pairs; i++) {
+VhostVDPAShared *shared = NULL;
+
+if (i) {
+shared = DO_UPCAST(VhostVDPAState, nc, ncs[0])->vhost_vdpa.shared;
+}
 ncs[i] = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name,
  vdpa_device_fd, i, 2, true, opts->x_svq,
- iova_range, features, errp);
+ iova_range, features, shared, errp);
 if (!ncs[i])
 goto err;
 }
 
 if (has_cvq) {
+VhostVDPAState *s0 = DO_UPCAST(VhostVDPAState, nc, ncs[0]);
+VhostVDPAShared *shared = s0->vhost_vdpa.shared;
+
 nc = net_vhost_vdpa_init(peer, TYPE_VHOST_VDPA, name,
  vdpa_device_fd, i, 1, false,
- opts->x_svq, iova_range, features, errp);
+ opts->x_svq, iova_range, features, shared,
+ errp);
 if (!nc)
 goto err;
 }
-- 
MST




[PULL 07/21] Fix bugs when VM shutdown with virtio-gpu unplugged

2023-12-26 Thread Michael S. Tsirkin
From: wangmeiling 

Virtio-gpu malloc memory for the queue when it realized, but the queues was not
released when it unrealized, which resulting in a memory leak. In addition,
vm_change_state_handler is not cleaned up, which is related to vdev and will
lead to segmentation fault when VM shutdown.

Signed-off-by: wangmeiling 
Signed-off-by: Binfeng Wu 
Message-Id: <7bbbc0f3-2ad9-83ca-b39b-f976d0837...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/display/virtio-gpu-base.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 37af256219..4fc7ef8896 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -251,7 +251,11 @@ void
 virtio_gpu_base_device_unrealize(DeviceState *qdev)
 {
 VirtIOGPUBase *g = VIRTIO_GPU_BASE(qdev);
+VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
 
+virtio_del_queue(vdev, 0);
+virtio_del_queue(vdev, 1);
+virtio_cleanup(vdev);
 migrate_del_blocker(&g->migration_blocker);
 }
 
-- 
MST




[PULL 02/21] tests: bios-tables-test: Rename smbios type 4 related test functions

2023-12-26 Thread Michael S. Tsirkin
From: Zhao Liu 

In fact, type4-count, core-count, core-count2, thread-count and
thread-count2 are tested with KVM not TCG.

Rename these test functions to reflect KVM base instead of TCG.

Signed-off-by: Zhao Liu 
Message-Id: <20231127160202.1037290-1-zhao1@linux.intel.com>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Igor Mammedov 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 tests/qtest/bios-tables-test.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index fe6a9a8563..21811a1ab5 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1015,7 +1015,7 @@ static void test_acpi_q35_tcg(void)
 free_test_data(&data);
 }
 
-static void test_acpi_q35_tcg_type4_count(void)
+static void test_acpi_q35_kvm_type4_count(void)
 {
 test_data data = {
 .machine = MACHINE_Q35,
@@ -1031,7 +1031,7 @@ static void test_acpi_q35_tcg_type4_count(void)
 free_test_data(&data);
 }
 
-static void test_acpi_q35_tcg_core_count(void)
+static void test_acpi_q35_kvm_core_count(void)
 {
 test_data data = {
 .machine = MACHINE_Q35,
@@ -1048,7 +1048,7 @@ static void test_acpi_q35_tcg_core_count(void)
 free_test_data(&data);
 }
 
-static void test_acpi_q35_tcg_core_count2(void)
+static void test_acpi_q35_kvm_core_count2(void)
 {
 test_data data = {
 .machine = MACHINE_Q35,
@@ -1065,7 +1065,7 @@ static void test_acpi_q35_tcg_core_count2(void)
 free_test_data(&data);
 }
 
-static void test_acpi_q35_tcg_thread_count(void)
+static void test_acpi_q35_kvm_thread_count(void)
 {
 test_data data = {
 .machine = MACHINE_Q35,
@@ -1082,7 +1082,7 @@ static void test_acpi_q35_tcg_thread_count(void)
 free_test_data(&data);
 }
 
-static void test_acpi_q35_tcg_thread_count2(void)
+static void test_acpi_q35_kvm_thread_count2(void)
 {
 test_data data = {
 .machine = MACHINE_Q35,
@@ -2262,15 +2262,15 @@ int main(int argc, char *argv[])
 qtest_add_func("acpi/q35/kvm/xapic", test_acpi_q35_kvm_xapic);
 qtest_add_func("acpi/q35/kvm/dmar", test_acpi_q35_kvm_dmar);
 qtest_add_func("acpi/q35/type4-count",
-   test_acpi_q35_tcg_type4_count);
+   test_acpi_q35_kvm_type4_count);
 qtest_add_func("acpi/q35/core-count",
-   test_acpi_q35_tcg_core_count);
+   test_acpi_q35_kvm_core_count);
 qtest_add_func("acpi/q35/core-count2",
-   test_acpi_q35_tcg_core_count2);
+   test_acpi_q35_kvm_core_count2);
 qtest_add_func("acpi/q35/thread-count",
-   test_acpi_q35_tcg_thread_count);
+   test_acpi_q35_kvm_thread_count);
 qtest_add_func("acpi/q35/thread-count2",
-   test_acpi_q35_tcg_thread_count2);
+   test_acpi_q35_kvm_thread_count2);
 }
 if (qtest_has_device("virtio-iommu-pci")) {
 qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
-- 
MST




[PULL 03/21] vhost: Add worker backend callouts

2023-12-26 Thread Michael S. Tsirkin
From: Mike Christie 

This adds the vhost backend callouts for the worker ioctls added in the
6.4 linux kernel commit:

c1ecd8e95007 ("vhost: allow userspace to create workers")

Signed-off-by: Mike Christie 
Reviewed-by: Stefano Garzarella 
Reviewed-by: Stefan Hajnoczi 

Message-Id: <20231204231618.21962-2-michael.chris...@oracle.com>
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/vhost-backend.h | 14 ++
 hw/virtio/vhost-backend.c | 28 
 2 files changed, 42 insertions(+)

diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h
index a86d103f82..70c2e8ffee 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -45,6 +45,8 @@ struct vhost_memory;
 struct vhost_vring_file;
 struct vhost_vring_state;
 struct vhost_vring_addr;
+struct vhost_vring_worker;
+struct vhost_worker_state;
 struct vhost_scsi_target;
 struct vhost_iotlb_msg;
 struct vhost_virtqueue;
@@ -85,6 +87,14 @@ typedef int (*vhost_set_vring_err_op)(struct vhost_dev *dev,
   struct vhost_vring_file *file);
 typedef int (*vhost_set_vring_busyloop_timeout_op)(struct vhost_dev *dev,
struct vhost_vring_state 
*r);
+typedef int (*vhost_attach_vring_worker_op)(struct vhost_dev *dev,
+struct vhost_vring_worker *worker);
+typedef int (*vhost_get_vring_worker_op)(struct vhost_dev *dev,
+ struct vhost_vring_worker *worker);
+typedef int (*vhost_new_worker_op)(struct vhost_dev *dev,
+   struct vhost_worker_state *worker);
+typedef int (*vhost_free_worker_op)(struct vhost_dev *dev,
+struct vhost_worker_state *worker);
 typedef int (*vhost_set_features_op)(struct vhost_dev *dev,
  uint64_t features);
 typedef int (*vhost_get_features_op)(struct vhost_dev *dev,
@@ -172,6 +182,10 @@ typedef struct VhostOps {
 vhost_set_vring_call_op vhost_set_vring_call;
 vhost_set_vring_err_op vhost_set_vring_err;
 vhost_set_vring_busyloop_timeout_op vhost_set_vring_busyloop_timeout;
+vhost_new_worker_op vhost_new_worker;
+vhost_free_worker_op vhost_free_worker;
+vhost_get_vring_worker_op vhost_get_vring_worker;
+vhost_attach_vring_worker_op vhost_attach_vring_worker;
 vhost_set_features_op vhost_set_features;
 vhost_get_features_op vhost_get_features;
 vhost_set_backend_cap_op vhost_set_backend_cap;
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 17f3fc6a08..833804dd40 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -158,6 +158,30 @@ static int vhost_kernel_set_vring_busyloop_timeout(struct 
vhost_dev *dev,
 return vhost_kernel_call(dev, VHOST_SET_VRING_BUSYLOOP_TIMEOUT, s);
 }
 
+static int vhost_kernel_new_worker(struct vhost_dev *dev,
+   struct vhost_worker_state *worker)
+{
+return vhost_kernel_call(dev, VHOST_NEW_WORKER, worker);
+}
+
+static int vhost_kernel_free_worker(struct vhost_dev *dev,
+struct vhost_worker_state *worker)
+{
+return vhost_kernel_call(dev, VHOST_FREE_WORKER, worker);
+}
+
+static int vhost_kernel_attach_vring_worker(struct vhost_dev *dev,
+struct vhost_vring_worker *worker)
+{
+return vhost_kernel_call(dev, VHOST_ATTACH_VRING_WORKER, worker);
+}
+
+static int vhost_kernel_get_vring_worker(struct vhost_dev *dev,
+ struct vhost_vring_worker *worker)
+{
+return vhost_kernel_call(dev, VHOST_GET_VRING_WORKER, worker);
+}
+
 static int vhost_kernel_set_features(struct vhost_dev *dev,
  uint64_t features)
 {
@@ -313,6 +337,10 @@ const VhostOps kernel_ops = {
 .vhost_set_vring_err = vhost_kernel_set_vring_err,
 .vhost_set_vring_busyloop_timeout =
 vhost_kernel_set_vring_busyloop_timeout,
+.vhost_get_vring_worker = vhost_kernel_get_vring_worker,
+.vhost_attach_vring_worker = vhost_kernel_attach_vring_worker,
+.vhost_new_worker = vhost_kernel_new_worker,
+.vhost_free_worker = vhost_kernel_free_worker,
 .vhost_set_features = vhost_kernel_set_features,
 .vhost_get_features = vhost_kernel_get_features,
 .vhost_set_backend_cap = vhost_kernel_set_backend_cap,
-- 
MST




[PULL 00/21] virtio,pc,pci: features, cleanups, fixes

2023-12-26 Thread Michael S. Tsirkin
The following changes since commit 80f1709aa0eb4de09b4240563463f991a5b9d855:

  Merge tag 'pull-loongarch-20231221' of https://gitlab.com/gaosong/qemu into 
staging (2023-12-21 19:44:19 -0500)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 7b67b2f0f4f7c5ec888a331af599d9daff735d60:

  vdpa: move memory listener to vhost_vdpa_shared (2023-12-25 11:34:55 -0500)


virtio,pc,pci: features, cleanups, fixes

vhost-scsi support for worker ioctls

fixes, cleanups all over the place.

Signed-off-by: Michael S. Tsirkin 


Aaron Young (1):
  hw/acpi: propagate vcpu hotplug after switch to modern interface

Dongli Zhang (1):
  vhost-scsi: fix usage of error_reportf_err()

Eugenio Pérez (14):
  vdpa: do not set virtio status bits if unneeded
  vdpa: add VhostVDPAShared
  vdpa: move iova tree to the shared struct
  vdpa: move iova_range to vhost_vdpa_shared
  vdpa: move shadow_data to vhost_vdpa_shared
  vdpa: use vdpa shared for tracing
  vdpa: move file descriptor to vhost_vdpa_shared
  vdpa: move iotlb_batch_begin_sent to vhost_vdpa_shared
  vdpa: move backend_cap to vhost_vdpa_shared
  vdpa: remove msg type of vhost_vdpa
  vdpa: move iommu_list to vhost_vdpa_shared
  vdpa: use VhostVDPAShared in vdpa_dma_map and unmap
  vdpa: use dev_shared in vdpa_iommu
  vdpa: move memory listener to vhost_vdpa_shared

Mathieu Poirier (1):
  virtio: rng: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

Mike Christie (2):
  vhost: Add worker backend callouts
  vhost-scsi: Add support for a worker thread per virtqueue

Zhao Liu (1):
  tests: bios-tables-test: Rename smbios type 4 related test functions

wangmeiling (1):
  Fix bugs when VM shutdown with virtio-gpu unplugged

 include/hw/virtio/vhost-backend.h |  14 
 include/hw/virtio/vhost-vdpa.h|  40 ++
 include/hw/virtio/virtio-scsi.h   |   1 +
 hw/acpi/cpu_hotplug.c |  20 -
 hw/display/virtio-gpu-base.c  |   4 +
 hw/scsi/vhost-scsi.c  |  66 ++-
 hw/scsi/vhost-user-scsi.c |   3 +-
 hw/virtio/vdpa-dev.c  |   7 +-
 hw/virtio/vhost-backend.c |  28 +++
 hw/virtio/vhost-user-rng.c|  16 
 hw/virtio/vhost-vdpa.c| 164 --
 net/vhost-vdpa.c  | 116 +--
 tests/qtest/bios-tables-test.c|  20 ++---
 hw/virtio/trace-events|  14 ++--
 14 files changed, 334 insertions(+), 179 deletions(-)




Re: [PATCH] target/riscv: Fix mcycle/minstret increment behavior

2023-12-26 Thread Michael Tokarev

26.12.2023 12:21, Daniel Henrique Barboza:

Michael,

This is a good candidate for qemu-trivial. Already acked.


Yeah, I've noticed it it earlier today and already copied to the
qemu-stable-toapply folder :)

Thanks!

/mjt



Re: [PATCH] target/riscv: Fix mcycle/minstret increment behavior

2023-12-26 Thread Daniel Henrique Barboza

Michael,

This is a good candidate for qemu-trivial. Already acked.


Thanks,

Daniel

On 12/26/23 01:05, Xu Lu wrote:

The mcycle/minstret counter's stop flag is mistakenly updated on a copy
on stack. Thus the counter increments even when the CY/IR bit in the
mcountinhibit register is set. This commit corrects its behavior.

Fixes: 3780e33732f88 (target/riscv: Support mcycle/minstret write operation)
Signed-off-by: Xu Lu 
---
  target/riscv/csr.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fde7ce1a5336..c50a33397c51 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -907,11 +907,11 @@ static int write_mhpmcounterh(CPURISCVState *env, int 
csrno, target_ulong val)
  static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong 
*val,
   bool upper_half, uint32_t ctr_idx)
  {
-PMUCTRState counter = env->pmu_ctrs[ctr_idx];
-target_ulong ctr_prev = upper_half ? counter.mhpmcounterh_prev :
- counter.mhpmcounter_prev;
-target_ulong ctr_val = upper_half ? counter.mhpmcounterh_val :
-counter.mhpmcounter_val;
+PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];
+target_ulong ctr_prev = upper_half ? counter->mhpmcounterh_prev :
+ counter->mhpmcounter_prev;
+target_ulong ctr_val = upper_half ? counter->mhpmcounterh_val :
+counter->mhpmcounter_val;
  
  if (get_field(env->mcountinhibit, BIT(ctr_idx))) {

  /*
@@ -919,12 +919,12 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState 
*env, target_ulong *val,
   * stop the icount counting. Just return the counter value written by
   * the supervisor to indicate that counter was not incremented.
   */
-if (!counter.started) {
+if (!counter->started) {
  *val = ctr_val;
  return RISCV_EXCP_NONE;
  } else {
  /* Mark that the counter has been stopped */
-counter.started = false;
+counter->started = false;
  }
  }
  




Re: [PATCH v11 0/7] Support x2APIC mode with TCG accelerator

2023-12-26 Thread Michael S. Tsirkin
On Mon, Dec 25, 2023 at 11:40:54PM +0700, Bui Quang Minh wrote:
> Hi everyone,
> 
> This series implements x2APIC mode in userspace local APIC and the
> RDMSR/WRMSR helper to access x2APIC registers in x2APIC mode. Intel iommu
> and AMD iommu are adjusted to support x2APIC interrupt remapping. With this
> series, we can now boot Linux kernel into x2APIC mode with TCG accelerator
> using either Intel or AMD iommu.
> 
> Testing to boot my own built Linux 6.3.0-rc2, the kernel successfully boot
> with enabled x2APIC and can enumerate CPU with APIC ID 257
> 
> Using Intel IOMMU
> 
> qemu/build/qemu-system-x86_64 \
>   -smp 2,maxcpus=260 \
>   -cpu qemu64,x2apic=on \
>   -machine q35 \
>   -device intel-iommu,intremap=on,eim=on \
>   -device qemu64-x86_64-cpu,x2apic=on,core-id=257,socket-id=0,thread-id=0 \
>   -m 2G \
>   -kernel $KERNEL_DIR \
>   -append "nokaslr console=ttyS0 root=/dev/sda earlyprintk=serial 
> net.ifnames=0" \
>   -drive file=$IMAGE_DIR,format=raw \
>   -nographic \
>   -s
> 
> Using AMD IOMMU
> 
> qemu/build/qemu-system-x86_64 \
>   -smp 2,maxcpus=260 \
>   -cpu qemu64,x2apic=on \
>   -machine q35 \
>   -device amd-iommu,intremap=on,xtsup=on \
>   -device qemu64-x86_64-cpu,x2apic=on,core-id=257,socket-id=0,thread-id=0 \
>   -m 2G \
>   -kernel $KERNEL_DIR \
>   -append "nokaslr console=ttyS0 root=/dev/sda earlyprintk=serial 
> net.ifnames=0" \
>   -drive file=$IMAGE_DIR,format=raw \
>   -nographic \
>   -s
> 
> Testing the emulated userspace APIC with kvm-unit-tests, disable test
> device with this patch

Seems to break build for windows/amd64
https://gitlab.com/mstredhat/qemu/-/pipelines/1118886361/failures





> diff --git a/lib/x86/fwcfg.c b/lib/x86/fwcfg.c
> index 1734afb..f56fe1c 100644
> --- a/lib/x86/fwcfg.c
> +++ b/lib/x86/fwcfg.c
> @@ -27,6 +27,7 @@ static void read_cfg_override(void)
> 
> if ((str = getenv("TEST_DEVICE")))
> no_test_device = !atol(str);
> +   no_test_device = true;
> 
> if ((str = getenv("MEMLIMIT")))
> fw_override[FW_CFG_MAX_RAM] = atol(str) * 1024 * 1024;
> 
> ~ env QEMU=/home/minh/Desktop/oss/qemu/build/qemu-system-x86_64 ACCEL=tcg \
> ./run_tests.sh -v -g apic
> 
> TESTNAME=apic-split TIMEOUT=90s ACCEL=tcg ./x86/run x86/apic.flat -smp 2
> -cpu qemu64,+x2apic,+tsc-deadline -machine kernel_irqchip=split FAIL
> apic-split (54 tests, 8 unexpected failures, 1 skipped)
> TESTNAME=ioapic-split TIMEOUT=90s ACCEL=tcg ./x86/run x86/ioapic.flat -smp
> 1 -cpu qemu64 -machine kernel_irqchip=split PASS ioapic-split (19 tests)
> TESTNAME=x2apic TIMEOUT=30 ACCEL=tcg ./x86/run x86/apic.flat -smp 2 -cpu
> qemu64,+x2apic,+tsc-deadline FAIL x2apic (54 tests, 8 unexpected failures,
> 1 skipped) TESTNAME=xapic TIMEOUT=60 ACCEL=tcg ./x86/run x86/apic.flat -smp
> 2 -cpu qemu64,-x2apic,+tsc-deadline -machine pit=off FAIL xapic (43 tests,
> 6 unexpected failures, 2 skipped)
> 
>   FAIL: apic_disable: *0xfee00030: 50014
>   FAIL: apic_disable: *0xfee00080: f0
>   FAIL: apic_disable: *0xfee00030: 50014
>   FAIL: apic_disable: *0xfee00080: f0
>   FAIL: apicbase: relocate apic
> 
> These errors are because we don't disable MMIO region when switching to
> x2APIC and don't support relocate MMIO region yet. This is a problem
> because, MMIO region is the same for all CPUs, in order to support these we
> need to figure out how to allocate and manage different MMIO regions for
> each CPUs. This can be an improvement in the future.
> 
>   FAIL: nmi-after-sti
>   FAIL: multiple nmi
> 
> These errors are in the way we handle CPU_INTERRUPT_NMI in core TCG.
> 
>   FAIL: TMCCT should stay at zero
> 
> This error is related to APIC timer which should be addressed in separate
> patch.
> 
> Version 11 changes,
> - Patch 2:
>   + Rebase to master and fix conflict with commit c04cfb4596 (hw/i386: fix
> short-circuit logic with non-optimizing builds)
> 
> Version 10 changes,
> - Patch 2:
>   + Fix null pointer dereference due to uninitialized local_apics when using
>   machine none
> - Patch 5, 7:
>   + These patches are added to follow the bios-tables-test instructions to
>   commit the new changed IVRS.ivrs binary file
> 
> Version 9 changes,
> - Patch 1:
>   + Create apic_msr_read/write which is a small wrapper around
>   apic_register_read/write that have additional x2apic mode check
> - Patch 2:
>   + Remove raise_exception_ra which is is TCG specific. Instead, return -1
>   and let the accelerator raise the appropriate exception
>   + Refactor apic_get_delivery_bitmask a little bit to reduce line length
>   + Move cpu_has_x2apic_feature and cpu_set_apic_feature from patch 3 to
>   patch 2 so that patch 2 can be compiled without patch 3
> - Patch 3:
>   + set_base in APICCommonClass now returns an int to indicate error
>   + Remove raise_exception_ra in apic_set base which is is TCG specific.
>   Instead, return -1 and let the accelerator raise the appropriate
>   exception
> 
> Version 8 changes,
> - Patch 2, 4:
>   + Rebase to master and resol

Re: [PATCH] target/riscv: Fix mcycle/minstret increment behavior

2023-12-26 Thread Daniel Henrique Barboza




On 12/26/23 01:05, Xu Lu wrote:

The mcycle/minstret counter's stop flag is mistakenly updated on a copy
on stack. Thus the counter increments even when the CY/IR bit in the
mcountinhibit register is set. This commit corrects its behavior.



Good catch.


Fixes: 3780e33732f88 (target/riscv: Support mcycle/minstret write operation)
Signed-off-by: Xu Lu 
---


Reviewed-by: Daniel Henrique Barboza 


  target/riscv/csr.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fde7ce1a5336..c50a33397c51 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -907,11 +907,11 @@ static int write_mhpmcounterh(CPURISCVState *env, int 
csrno, target_ulong val)
  static RISCVException riscv_pmu_read_ctr(CPURISCVState *env, target_ulong 
*val,
   bool upper_half, uint32_t ctr_idx)
  {
-PMUCTRState counter = env->pmu_ctrs[ctr_idx];
-target_ulong ctr_prev = upper_half ? counter.mhpmcounterh_prev :
- counter.mhpmcounter_prev;
-target_ulong ctr_val = upper_half ? counter.mhpmcounterh_val :
-counter.mhpmcounter_val;
+PMUCTRState *counter = &env->pmu_ctrs[ctr_idx];
+target_ulong ctr_prev = upper_half ? counter->mhpmcounterh_prev :
+ counter->mhpmcounter_prev;
+target_ulong ctr_val = upper_half ? counter->mhpmcounterh_val :
+counter->mhpmcounter_val;
  
  if (get_field(env->mcountinhibit, BIT(ctr_idx))) {

  /*
@@ -919,12 +919,12 @@ static RISCVException riscv_pmu_read_ctr(CPURISCVState 
*env, target_ulong *val,
   * stop the icount counting. Just return the counter value written by
   * the supervisor to indicate that counter was not incremented.
   */
-if (!counter.started) {
+if (!counter->started) {
  *val = ctr_val;
  return RISCV_EXCP_NONE;
  } else {
  /* Mark that the counter has been stopped */
-counter.started = false;
+counter->started = false;
  }
  }