Re: [PATCH 0/4] update-linux-headers: prepare for updating to 6.9+ and for SNP patches

[Cornelia Huck]

On Mon, Jun 03 2024, Paolo Bonzini  wrote:

> This series contains a few cleanups and fixes to update-linux-headers.sh,
> which I needed or found in order to pass CI for the SEV-SNP patches.
>
> First, updating linux-headers/ fails due to the lack of
> arch/loongarch/include/uapi/asm/bitsperlong.h.  I am not sure if I am
> doing something wrong but it is caused by commit 3efc75ad9d9
> ("scripts/update-linux-headers.sh: Remove temporary directory inbetween",
> 2024-05-29); if so, I guess I'm 1-1 with Thomas given my own bug in
> commit 66210a1a30 that he fixed.
>
> Before commit 3efc75ad9d9, the missing file would incorrectly cause stale
> files to be included in linux-headers/.  The files were never committed
> to qemu.git, but were wrong nevertheless. The build would just use
> the system version of the files, which is opposite to the idea of
> importing Linux header files into QEMU's tree.
>
> Second, pvpanic.h was incorrectly included in a slightly different
> path than what is used in Linux.
>
> Third, SNP host-side patches will need linux/kvm_para.h, because some
> of the supported distros do not have a definition for KVM_HC_MAP_GPA_RANGE.
> Including it is a bit complicated because we also have a version of
> x86's kvm_para.h under include/standard-headers/; linux/kvm_para.h
> tries to include  and that could cause conflicts.
> So, the linux/kvm_para.h is also placed in include/standard-headers
> (patch 4).
>
> Pankaj Gupta (1):
>   linux-headers: Update to current kvm/next
>
> Paolo Bonzini (3):
>   update-linux-headers: fix forwarding to asm-generic headers
>   update-linux-headers: move pvpanic.h to correct directory
>   update-linux-headers: import linux/kvm_para.h header
>
>  include/standard-headers/linux/kvm_para.h | 38 ++
>  .../{linux => misc}/pvpanic.h |  0
>  linux-headers/asm-loongarch/kvm.h |  4 ++
>  linux-headers/asm-riscv/kvm.h |  1 +
>  linux-headers/asm-x86/kvm.h   | 52 ++-
>  linux-headers/asm-x86/kvm_para.h  |  1 +
>  linux-headers/linux/kvm_para.h|  2 +
>  linux-headers/linux/vhost.h   | 15 +++---
>  hw/misc/pvpanic-isa.c |  2 +-
>  hw/misc/pvpanic-pci.c |  2 +-
>  hw/misc/pvpanic.c |  2 +-
>  scripts/update-linux-headers.sh   | 37 +++--
>  12 files changed, 141 insertions(+), 15 deletions(-)
>  create mode 100644 include/standard-headers/linux/kvm_para.h
>  rename include/standard-headers/{linux => misc}/pvpanic.h (100%)
>  create mode 100644 linux-headers/asm-x86/kvm_para.h
>  create mode 100644 linux-headers/linux/kvm_para.h

With the hash added to the headers update,

Reviewed-by: Cornelia Huck 

for the series.

Re: [PATCH 3/4] linux-headers: Update to current kvm/next

[Cornelia Huck]

On Mon, Jun 03 2024, Paolo Bonzini  wrote:

> On Mon, Jun 3, 2024 at 5:58 PM Cornelia Huck  wrote:
>> Hm, I'm not sure updating to kvm/next is a good idea ("current kvm/next"
>> does not mean anything without a commit hash anyway.) I think we should
>> only update to something that's in Linus' tree already... how stable is
>> kvm/next?
>
> It is stable, things are only applied there once UAPI is set. Even
> rebasing is very rare.
>
> The problem here is that if (as is the case for 6.11) the merge window
> only opens once QEMU is in freeze, waiting for it would delay merging
> the QEMU side by 4 months. In this case, the patches barely missed
> 6.10.

If we're confident that it's stable, can we please mention a hash?
"current" is not very descriptive :)

Re: [PATCH 3/4] linux-headers: Update to current kvm/next

[Cornelia Huck]

On Mon, Jun 03 2024, Paolo Bonzini  wrote:

> From: Pankaj Gupta 
>
> Also brings in an linux-headers/linux/vhost.h fix from v6.9-rc4.
>
> Co-developed-by: Michael Roth 
> Signed-off-by: Michael Roth 
> Signed-off-by: Pankaj Gupta 
> Message-ID: <20240530111643.1091816-3-pankaj.gu...@amd.com>
> Signed-off-by: Paolo Bonzini 
> ---
>  linux-headers/asm-loongarch/kvm.h |  4 +++
>  linux-headers/asm-riscv/kvm.h |  1 +
>  linux-headers/asm-x86/kvm.h   | 52 ++-
>  linux-headers/linux/vhost.h   | 15 -
>  4 files changed, 64 insertions(+), 8 deletions(-)

Hm, I'm not sure updating to kvm/next is a good idea ("current kvm/next"
does not mean anything without a commit hash anyway.) I think we should
only update to something that's in Linus' tree already... how stable is
kvm/next?

Re: [PATCH 0/2] ppc: spapr: Nested kvm guest migration fixes

[Cornelia Huck]

On Mon, Jun 03 2024, Shivaprasad G Bhat  wrote:

> The series fixes the issues exposed by the kvm-unit-tests[1]
> sprs-migration test.
>
> The sprs DEXCR and HASKKEYR are not registered with one-reg IDs
> without which the Qemu is not setting them to their 'previous'
> value during guest migreation at destination.
>
> The two patches in the series take care of this. Also, the PPC
> kvm header changes are selectively picked for the required
> definitions posted here at [2].
>
> References:
> [1]: https://github.com/kvm-unit-tests/kvm-unit-tests
> [2]: 
> https://lore.kernel.org/kvm/171741323521.6631.11242552089199677395.st...@linux.ibm.com
>
> ---
>
> Shivaprasad G Bhat (2):
>   target/ppc/cpu_init: Synchronize DEXCR with KVM for migration
>   target/ppc/cpu_init: Synchronize HASHKEYR with KVM for migration
>
>
>  linux-headers/asm-powerpc/kvm.h | 2 ++

Please split out any changes under linux-headers/ into a separate patch;
those files need to be changed via a full header update against a
released kernel version. If the changes are not yet upstream in the
Linux kernel, please put in a clearly marked placeholder patch in the
meanwhile.


>  target/ppc/cpu_init.c   | 8 
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> --
> Signature

Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween

[Cornelia Huck]

On Mon, May 27 2024, Thomas Huth  wrote:

> On 27/05/2024 17.04, Cornelia Huck wrote:
>> On Mon, May 27 2024, Thomas Huth  wrote:
>> 
>>> We are reusing the same temporary directory for installing the headers
>>> of all targets, so there could be stale files here when switching from
>>> one target to another. Make sure to delete the folder before installing
>>> a new set of target headers into it.
>>>
>>> Signed-off-by: Thomas Huth 
>>> ---
>>>   scripts/update-linux-headers.sh | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/scripts/update-linux-headers.sh 
>>> b/scripts/update-linux-headers.sh
>>> index 8963c39189..fbf7e119bc 100755
>>> --- a/scripts/update-linux-headers.sh
>>> +++ b/scripts/update-linux-headers.sh
>>> @@ -112,6 +112,7 @@ for arch in $ARCHLIST; do
>>>   arch_var=ARCH
>>>   fi
>>>   
>>> +rm -rf "$hdrdir"
>>>   make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" 
>>> $arch_var=$arch headers_install
>>>   
>>>   rm -rf "$output/linux-headers/asm-$arch"
>> 
>> Hm. I presume that headers-install gives us the same set of headers
>> outside include/asm for every arch?
>
> I just double-checked and yes, apart from a file called "a.out.h", the other 
> headers outside of the asm subfolder are the same. So AFAICS there is still 
> a slight chance that we might pick up a wrong file from the asm folder... 
> shall I change the patch to only delete that subfolder? Or is that chance 
> just too small that we can ignore it?

The asm folder is probably fine, but there's a chance for errors; if
moving from "use union of files for all archs outside asm" to "use files
for the last arch in the list outside asm" always results in the same
set of files, your patch is probably the better choice here.

Acked-by: Cornelia Huck 

Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween

[Cornelia Huck]

On Mon, May 27 2024, Thomas Huth  wrote:

> We are reusing the same temporary directory for installing the headers
> of all targets, so there could be stale files here when switching from
> one target to another. Make sure to delete the folder before installing
> a new set of target headers into it.
>
> Signed-off-by: Thomas Huth 
> ---
>  scripts/update-linux-headers.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
> index 8963c39189..fbf7e119bc 100755
> --- a/scripts/update-linux-headers.sh
> +++ b/scripts/update-linux-headers.sh
> @@ -112,6 +112,7 @@ for arch in $ARCHLIST; do
>  arch_var=ARCH
>  fi
>  
> +rm -rf "$hdrdir"
>  make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" $arch_var=$arch 
> headers_install
>  
>  rm -rf "$output/linux-headers/asm-$arch"

Hm. I presume that headers-install gives us the same set of headers
outside include/asm for every arch?

Re: [PATCH] scripts/update-linux-headers.sh: Fix the path of setup_data.h

[Cornelia Huck]

On Mon, May 27 2024, Thomas Huth  wrote:

> When running the update-linx-headers.sh script, it currently fails with:
>
> scripts/update-linux-headers.sh: line 73: 
> .../qemu/standard-headers/asm-x86/setup_data.h: No such file or directory
>
> The "include" folder is obviously missing here - no clue how this could
> have worked before?

Presumably nobody tried to run the script against a 6.9-ish kernel?

>
> Fixes: 66210a1a30 ("scripts/update-linux-headers: Add setup_data.h to import 
> list")
> Signed-off-by: Thomas Huth 
> ---
>  scripts/update-linux-headers.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
> index fbf7e119bc..23afe8c08a 100755
> --- a/scripts/update-linux-headers.sh
> +++ b/scripts/update-linux-headers.sh
> @@ -159,7 +159,7 @@ for arch in $ARCHLIST; do
>  cp_portable "$hdrdir/bootparam.h" \
>  "$output/include/standard-headers/asm-$arch"
>  cp_portable "$hdrdir/include/asm/setup_data.h" \
> -"$output/standard-headers/asm-x86"
> +"$output/include/standard-headers/asm-x86"
>  fi
>  if [ $arch = riscv ]; then
>  cp "$hdrdir/include/asm/ptrace.h" "$output/linux-headers/asm-riscv/"

Reviewed-by: Cornelia Huck 

Re: [PATCH v9] arm/kvm: Enable support for KVM_ARM_VCPU_PMU_V3_FILTER

[Cornelia Huck]

On Wed, Apr 10 2024, Thomas Huth  wrote:

> On 09/04/2024 09.47, Shaoqin Huang wrote:
>> Hi Thmoas,
>> 
>> On 4/9/24 13:33, Thomas Huth wrote:
 +    assert_has_feature(qts, "host", "kvm-pmu-filter");
>>>
>>> So you assert here that the feature is available ...
>>>
   assert_has_feature(qts, "host", "kvm-steal-time");
   assert_has_feature(qts, "host", "sve");
   resp = do_query_no_props(qts, "host");
 +    kvm_supports_pmu_filter = resp_get_feature_str(resp, 
 "kvm-pmu-filter");
   kvm_supports_steal_time = resp_get_feature(resp, 
 "kvm-steal-time");
   kvm_supports_sve = resp_get_feature(resp, "sve");
   vls = resp_get_sve_vls(resp);
   qobject_unref(resp);
 +    if (kvm_supports_pmu_filter) { >
>>> ... why do you then need to check for its availability here again?
>>> I either don't understand this part of the code, or you could drop the 
>>> kvm_supports_pmu_filter variable and simply always execute the code below.
>> 
>> Thanks for your reviewing. I did so because all other feature like 
>> "kvm-steal-time" check its availability again. I don't know the original 
>> reason why they did that. I just followed it.
>> 
>> Do you think we should delete all the checking?
>
> resp_get_feature() seems to return a boolean value, so though these feature 
> could be there, they still could be disabled, I assume? Thus we likely need 
> to keep the check for those.

This had confused me as well when I looked at it the last time -- one
thing is to check whether we have a certain prop in the cpu model, the
other one whether we actually support it. Maybe this needs some
comments?

Re: [PATCH] linux-headers: change the annotation of VFIO_IOMMU_SPAPR_REGISTER_MEMORY in vfio.h

[Cornelia Huck]

On Thu, Apr 11 2024, JianChunfu  wrote:

> The ioctl(VFIO_IOMMU_MAP_DMA/VFIO_IOMMU_UNMAP_DMA) won't be called
> in SPAPR machine, which is replaced by VFIO_IOMMU_SPAPR_TCE_CREATE/
> VFIO_IOMMU_SPAPR_TCE_REMOVE, so change the description.
>
> Signed-off-by: JianChunfu 
> ---
>  linux-headers/linux/vfio.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Instead of editing things under linux-headers directly, the kernel
source file needs to be changed instead so that a headers sync will
update the file here. [No opinion on the actual change.]

Re: [PATCH] Revert "hw/virtio: Add support for VDPA network simulation devices"

[Cornelia Huck]

On Mon, Apr 08 2024, "Michael S. Tsirkin"  wrote:

> This reverts commit cd341fd1ffded978b2aa0b5309b00be7c42e347c.
>
> The patch adds non-upstream code in
> include/standard-headers/linux/virtio_pci.h
> which would make maintainance harder.
>
> Revert for now.
>
> Suggested-by: Jason Wang 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  include/hw/virtio/virtio-pci.h  |   5 -
>  include/hw/virtio/virtio.h  |  19 --
>  include/standard-headers/linux/virtio_pci.h |   7 -
>  hw/net/virtio-net.c |  16 --
>  hw/virtio/virtio-pci.c  | 189 +---
>  hw/virtio/virtio.c  |  39 
>  MAINTAINERS |   5 -
>  docs/system/device-emulation.rst|   1 -
>  docs/system/devices/vdpa-net.rst| 121 -
>  9 files changed, 3 insertions(+), 399 deletions(-)
>  delete mode 100644 docs/system/devices/vdpa-net.rst

Acked-by: Cornelia Huck 

We should get rid of this before we release a version with non-upstream
header code.

Re: [PATCH for-9.1 v5 1/3] hw: Add compat machines for 9.1

[Cornelia Huck]

On Mon, Mar 25 2024, Paolo Bonzini  wrote:

> Add 9.1 machine types for arm/i440fx/m68k/q35/s390x/spapr.
>
> Cc: Cornelia Huck 
> Cc: Thomas Huth 
> Cc: Harsh Prateek Bora 
> Cc: Gavin Shan 
> Signed-off-by: Paolo Bonzini 
> ---
>  include/hw/boards.h|  3 +++
>  include/hw/i386/pc.h   |  3 +++
>  hw/arm/virt.c  | 11 +--
>  hw/core/machine.c  |  3 +++
>  hw/i386/pc.c   |  3 +++
>  hw/i386/pc_piix.c  | 17 ++---
>  hw/i386/pc_q35.c   | 14 --
>  hw/m68k/virt.c | 11 +--
>  hw/ppc/spapr.c | 17 ++---
>  hw/s390x/s390-virtio-ccw.c | 14 +-
>  10 files changed, 83 insertions(+), 13 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [PATCH 09/10] hw/pci: Set write-mask bits for PCIE Link-Control-2 register

[Cornelia Huck]

On Mon, Mar 25 2024, Cédric Le Goater  wrote:

> On 3/21/24 11:04, Saif Abrar wrote:
>> PHB updates the register PCIE Link-Control-2.
>> Set the write-mask bits for TLS, ENTER_COMP, TX_MARGIN,
>> HASD, MOD_COMP, COMP_SOS and COMP_P_DE.
>
>
> You should resend this patch independently of the PowerNV PHB changes.
>
>
> Thanks,
>
> C.
>
>
>
>> Signed-off-by: Saif Abrar 
>> ---
>>   hw/pci/pcie.c | 6 ++
>>   include/standard-headers/linux/pci_regs.h | 3 +++
>>   2 files changed, 9 insertions(+)

This patch also needs to be split: the code under standard-headers/ is
updated via a Linux headers update, which should either be a full update
against a released kernel version (can be -rc), or a placeholder patch
if the changes have not yet been merged.

Re: [PATCH vhost v4 1/6] virtio_balloon: remove the dependence where names[] is null

[Cornelia Huck]

On Mon, Mar 25 2024, Xuan Zhuo  wrote:

> On Fri, 22 Mar 2024 22:02:27 +0100, David Hildenbrand  
> wrote:
>> On 22.03.24 20:16, Daniel Verkamp wrote:
>> > On Thu, Mar 21, 2024 at 3:16 AM Xuan Zhuo  
>> > wrote:
>> >>
>> >> Currently, the init_vqs function within the virtio_balloon driver relies
>> >> on the condition that certain names array entries are null in order to
>> >> skip the initialization of some virtual queues (vqs). This behavior is
>> >> unique to this part of the codebase. In an upcoming commit, we plan to
>> >> eliminate this dependency by removing the function entirely. Therefore,
>> >> with this change, we are ensuring that the virtio_balloon no longer
>> >> depends on the aforementioned function.
>> >
>> > This is a behavior change, and I believe means that the driver no
>> > longer follows the spec [1].
>> >
>> > For example, the spec says that virtqueue 4 is reporting_vq, and
>> > reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING is set,
>> > but there is no mention of its virtqueue number changing if other
>> > features are not set. If a device/driver combination negotiates
>> > VIRTIO_BALLOON_F_PAGE_REPORTING but not VIRTIO_BALLOON_F_STATS_VQ or
>> > VIRTIO_BALLOON_F_FREE_PAGE_HINT, my reading of the specification is
>> > that reporting_vq should still be vq number 4, and vq 2 and 3 should
>> > be unused. This patch would make the reporting_vq use vq 2 instead in
>> > this case.
>> >
>> > If the new behavior is truly intended, then the spec does not match
>> > reality, and it would need to be changed first (IMO); however,
>> > changing the spec would mean that any devices implemented correctly
>> > per the previous spec would now be wrong, so some kind of mechanism
>> > for detecting the new behavior would be warranted, e.g. a new
>> > non-device-specific virtio feature flag.
>> >
>> > I have brought this up previously on the virtio-comment list [2], but
>> > it did not receive any satisfying answers at that time.

I had missed it back then, but now that I read it, I realize that we
really have a bit of a mess here :/

>>
>> Rings a bell, but staring at this patch, I thought that there would be
>> no behavioral change. Maybe I missed it :/
>>
>> I stared at virtio_ccw_find_vqs(), and it contains:
>>
>>  for (i = 0; i < nvqs; ++i) {
>>  if (!names[i]) {
>>  vqs[i] = NULL;
>>  continue;
>>  }
>>
>>  vqs[i] = virtio_ccw_setup_vq(vdev, queue_idx++, callbacks[i],
>>   names[i], ctx ? ctx[i] : false,
>>   ccw);
>>  if (IS_ERR(vqs[i])) {
>>  ret = PTR_ERR(vqs[i]);
>>  vqs[i] = NULL;
>>  goto out;
>>  }
>>  }
>>
>> We increment queue_idx only if an entry was not NULL. SO I thought no
>> behavioral change? (at least on s390x :) )

The code for pci behaves in the same way.

>>
>> It's late here in Germany, so maybe I'm missing something.
>
> I think we've encountered a tricky issue. Currently, all transports handle 
> queue
> id by incrementing them in order, without skipping any queue id. So, I'm quite
> surprised that my changes would affect the spec. The fact that the
> 'names' value is null is just a small trick in the Linux kernel implementation
> and should not have an impact on the queue id.
>
> I believe that my recent modification will not affect the spec. So, let's
> consider the issues with this patch set separately for now. Regarding the 
> Memory
> Balloon Device, it has been operational for many years, and perhaps we should
> add to the spec that if a certain vq does not exist, then subsequent vqs will
> take over its id.

The changes here do not really seem to affect the spec issue that Daniel
had noted, unless I'm reading the code wrong.

However, we should try to address the spec mess, where we have at least
some of the most popular/important implementations behaving differently
than the spec describes... I would suggest to discuss that on the virtio
lists -- but they are still dead, and at this point I'm just hoping
they'll come back eventually :/

Re: [PATCH vhost v4 1/6] virtio_balloon: remove the dependence where names[] is null

[Cornelia Huck]

On Mon, Mar 25 2024, Xuan Zhuo  wrote:

> On Fri, 22 Mar 2024 22:02:27 +0100, David Hildenbrand  
> wrote:
>> On 22.03.24 20:16, Daniel Verkamp wrote:
>> > On Thu, Mar 21, 2024 at 3:16 AM Xuan Zhuo  
>> > wrote:
>> >>
>> >> Currently, the init_vqs function within the virtio_balloon driver relies
>> >> on the condition that certain names array entries are null in order to
>> >> skip the initialization of some virtual queues (vqs). This behavior is
>> >> unique to this part of the codebase. In an upcoming commit, we plan to
>> >> eliminate this dependency by removing the function entirely. Therefore,
>> >> with this change, we are ensuring that the virtio_balloon no longer
>> >> depends on the aforementioned function.
>> >
>> > This is a behavior change, and I believe means that the driver no
>> > longer follows the spec [1].
>> >
>> > For example, the spec says that virtqueue 4 is reporting_vq, and
>> > reporting_vq only exists if VIRTIO_BALLOON_F_PAGE_REPORTING is set,
>> > but there is no mention of its virtqueue number changing if other
>> > features are not set. If a device/driver combination negotiates
>> > VIRTIO_BALLOON_F_PAGE_REPORTING but not VIRTIO_BALLOON_F_STATS_VQ or
>> > VIRTIO_BALLOON_F_FREE_PAGE_HINT, my reading of the specification is
>> > that reporting_vq should still be vq number 4, and vq 2 and 3 should
>> > be unused. This patch would make the reporting_vq use vq 2 instead in
>> > this case.
>> >
>> > If the new behavior is truly intended, then the spec does not match
>> > reality, and it would need to be changed first (IMO); however,
>> > changing the spec would mean that any devices implemented correctly
>> > per the previous spec would now be wrong, so some kind of mechanism
>> > for detecting the new behavior would be warranted, e.g. a new
>> > non-device-specific virtio feature flag.
>> >
>> > I have brought this up previously on the virtio-comment list [2], but
>> > it did not receive any satisfying answers at that time.

I had missed it back then, but now that I read it, I realize that we
really have a bit of a mess here :/

>>
>> Rings a bell, but staring at this patch, I thought that there would be
>> no behavioral change. Maybe I missed it :/
>>
>> I stared at virtio_ccw_find_vqs(), and it contains:
>>
>>  for (i = 0; i < nvqs; ++i) {
>>  if (!names[i]) {
>>  vqs[i] = NULL;
>>  continue;
>>  }
>>
>>  vqs[i] = virtio_ccw_setup_vq(vdev, queue_idx++, callbacks[i],
>>   names[i], ctx ? ctx[i] : false,
>>   ccw);
>>  if (IS_ERR(vqs[i])) {
>>  ret = PTR_ERR(vqs[i]);
>>  vqs[i] = NULL;
>>  goto out;
>>  }
>>  }
>>
>> We increment queue_idx only if an entry was not NULL. SO I thought no
>> behavioral change? (at least on s390x :) )

The code for pci behaves in the same way.

>>
>> It's late here in Germany, so maybe I'm missing something.
>
> I think we've encountered a tricky issue. Currently, all transports handle 
> queue
> id by incrementing them in order, without skipping any queue id. So, I'm quite
> surprised that my changes would affect the spec. The fact that the
> 'names' value is null is just a small trick in the Linux kernel implementation
> and should not have an impact on the queue id.
>
> I believe that my recent modification will not affect the spec. So, let's
> consider the issues with this patch set separately for now. Regarding the 
> Memory
> Balloon Device, it has been operational for many years, and perhaps we should
> add to the spec that if a certain vq does not exist, then subsequent vqs will
> take over its id.

The changes here do not really seem to affect the spec issue that Daniel
had noted, unless I'm reading the code wrong.

However, we should try to address the spec mess, where we have at least
some of the most popular/important implementations behaving differently
than the spec describes... I would suggest to discuss that on the virtio
lists -- but they are still dead, and at this point I'm just hoping
they'll come back eventually :/

Re: [virtio-dev] Re: [virtio-comment] [RFC PATCH v3] virtio-can: Device specification.

[Cornelia Huck]

On Wed, Feb 21 2024, Marc Kleine-Budde  wrote:

> On 21.02.2024 14:16:54, Matias Ezequiel Vara Larsen wrote:
>> On Wed, Feb 21, 2024 at 01:49:31PM +0100, Marc Kleine-Budde wrote:
>> > On 21.02.2024 11:37:58, Matias Ezequiel Vara Larsen wrote:
>> > > > > +The length of the \field{sdu} is determined by the \field{length}.
>> > > > > +
>> > > > > +The type of a CAN message identifier is determined by 
>> > > > > \field{flags}. The
>> > > > > +3 most significant bits of \field{can_id} do not bear the 
>> > > > > information
>> > > > > +about the type of the CAN message identifier and are 0.
>> > > > > +
>> > > > > +The device MUST reject any CAN frame type for which support has not 
>> > > > > been
>> > > > > +negotiated with VIRTIO_CAN_RESULT_NOT_OK in \field{result} and MUST 
>> > > > > NOT
>> > > > > +schedule the message for transmission. A CAN frame with an 
>> > > > > undefined bit
>> > > > > +set in \field{flags} is treated like a CAN frame for which support 
>> > > > > has
>> > > > > +not been negotiated.
>> > > > > +
>> > > > > +The device MUST reject any CAN frame for which \field{can_id} or
>> > > > > +\field{sdu} length are out of range or the CAN controller is in an
>> > > > > +invalid state with VIRTIO_CAN_RESULT_NOT_OK in \field{result} and 
>> > > > > MUST
>> > > > > +NOT schedule the message for transmission.
>> > > > > +
>> > > I am not very familiar with CAN but how does the device figure out that
>> > > the can_id is out of range?
>> > 
>> > In classical CAN we have the standard CAN frames, which have an 11 bit
>> > ID, and there are extended CAN frames, which have 29 bits ID. Extended
>> > frames are signaled with VIRTIO_CAN_FLAGS_EXTENDED set.
>> > 
>> > So if a standard frame uses more than 11 Bits of CAN-ID, it's considered
>> > out of range.
>
> Another option would be an extended frame (VIRTIO_CAN_FLAGS_EXTENDED
> set) and using more than 29 bits.
>
>> Thanks Marc for the explanation. Do you think that it would be
>> worthwhile to add that to the spec at some point?
>
> Yes that makes sense as it clarifies what's meant by out of range for
> CAN-IDs, for the valid length a reference to
> \item[VIRTIO_CAN_F_CAN_CLASSIC (0)] and \item[VIRTIO_CAN_F_CAN_FD (1)]
> might be added.

[virtio mailing lists are supposedly down for migration right now, I
hope there's some kind of backfill happening later...]

If the question comes up, it does make sense to add a
clarification... as the virtio-can spec is already voted upon and
merged, we'd need a patch on top. Not sure if it would qualify as an
editorial update or a vote would be needed, best to see it first :)


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[hpx-commits] [STEllAR-GROUP/hpx] 9974bb: Update CMakeLists.txt

[Kevin Huck]

  Branch: refs/heads/khuck-patch-1
  Home:   https://github.com/STEllAR-GROUP/hpx
  Commit: 9974bb5739b822c7924e98c233700026438bd08b
  
https://github.com/STEllAR-GROUP/hpx/commit/9974bb5739b822c7924e98c233700026438bd08b
  Author: Kevin Huck 
  Date:   2024-02-22 (Thu, 22 Feb 2024)

  Changed paths:
M CMakeLists.txt

  Log Message:
  ---
  Update CMakeLists.txt

Updating APEX version to latest release



To unsubscribe from these emails, change your notification settings at 
https://github.com/STEllAR-GROUP/hpx/settings/notifications
___
hpx-commits mailing list
hpx-commits@mail.cct.lsu.edu
https://mail.cct.lsu.edu/mailman/listinfo/hpx-commits

Re: [virtio-dev] [RFC PATCH v3] virtio-can: Device specification.

[Cornelia Huck]

On Fri, Jun 09 2023, Mikhail Golubev-Ciuchea 
 wrote:

> From: Harald Mommer 
>
> virtio-can is a virtual CAN device. It provides a way to give access to
> a CAN controller from a driver guest. The device is aimed to be used by
> driver guests running a HLOS as well as by driver guests running a
> typical RTOS as used in controller environments.
>
> Signed-off-by: Harald Mommer 
> Signed-off-by: Mikhail Golubev-Ciuchea 
> 

(...)

> diff --git a/device-types/can/device-conformance.tex 
> b/device-types/can/device-conformance.tex
> new file mode 100644
> index 000..f944ffd
> --- /dev/null
> +++ b/device-types/can/device-conformance.tex
> @@ -0,0 +1,8 @@
> +\conformance{\subsection}{CAN Device Conformance}\label{sec:Conformance / 
> Device Conformance / CAN Device Conformance}
> +
> +A CAN device MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{devicenormative:Device Types / CAN Device / Feature bits}

I just noticed that this introduces[1] an undefined reference (there's no
device normative section for feature bits.) I'd suggest to just drop
this reference (I can do that as a simple editorial update); if we want
to have an actual normative reference for feature bits, it should be
done via an extra change on top.

> +\item \ref{devicenormative:Device Types / CAN Device / Device Operation / 
> CAN Message Transmission}
> +\end{itemize}

[1] Easy to miss, as the current 1.4 branch has a pre-existing undefined
reference, which will go away once we finally can merge into the main
branch again and have all that craziness go away after we manage to do
the actual 1.3 release :/


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] [RFC PATCH v3 0/3] Virtio SPI Linux driver compliant to draft spec V10

[Cornelia Huck]

On Tue, Feb 13 2024, Harald Mommer  wrote:

> This is the 3rd RFC version of a virtio SPI Linux driver which is
> intended to be compliant with the proposed virtio SPI draft
> specification V10.

FWIW: this version of the SPI spec has been voted in for virtio 1.4 (and
is consequently available on the virtio-1.4 branch of the virtio spec.)
For all intents and purposes, this makes this spec final (modulo
possible future extensions).


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH] sound: add sampling rates 12000Hz and 24000Hz

[Cornelia Huck]

On Wed, Feb 14 2024, Anton Yakovlev  wrote:

> Thanks Pape!
>
>
> On 05.02.2024 23:41, Pape, Andreas (ADITG/ESS3) wrote:
>> 24kHz is used for 'super wideband' voice transmission 12kHz is added 'for 
>> completeness'
>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/184
>> 
>> Signed-off-by: Andreas Pape 
>
> Reviewed-by: Anton Yakovlev 

Thanks for re-adding your R-b!

>
>> ---
>>   device-types/sound/description.tex | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] [RFC PATCH v3] virtio-can: Device specification.

[Cornelia Huck]

On Fri, Jun 09 2023, Mikhail Golubev-Ciuchea 
 wrote:

> diff --git a/introduction.tex b/introduction.tex
> index b7155bf..d560c63 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -101,6 +101,8 @@ \section{Normative References}\label{sec:Normative 
> References}
>   \phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
>  Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic 
> Cureve Cryptography'', Version 1.0, September 2000.
>   \newline\url{https://www.secg.org/sec1-v2.pdf}\\
> + \phantomsection\label{intro:CAN}\textbf{[CAN]} &
> +ISO 11898-1:2015 Road vehicles -- Controller area network (CAN) -- Part 
> 1: Data link layer and physical signalling\\

Just noticed while massaging this patch to apply on top of the 1.4
branch: This reference does not include an url; I guess
https://www.iso.org/standard/63648.html would be the place to point to?

(This can easily be added via an editorial update on top.)


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH V2] virtio-transport: Clarify requirements

[Cornelia Huck]

On Tue, Jan 30 2024, Viresh Kumar  wrote:

> The virtio documentation currently doesn't define any generic
> requirements that are applicable to all transports. They can be useful
> while adding support for a new transport.
>
> This commit tries to define the same.
>
> Signed-off-by: Viresh Kumar 
> ---
> V1->V2:
> - Lot of changes after discussions with Alex and Cornelia.
> - Almost a rewrite of the first commit.
> - Add Transport normative sections.
>
>  commands.tex|  1 +
>  conformance.tex | 14 +
>  content.tex | 78 +++--
>  3 files changed, 91 insertions(+), 2 deletions(-)
>
> diff --git a/commands.tex b/commands.tex
> index 25ea8ee3bc78..692ef0833a88 100644
> --- a/commands.tex
> +++ b/commands.tex
> @@ -8,6 +8,7 @@
>  \newcommand{\field}[1]{\emph{#1}}
>  
>  % Mark a normative section (driver or device)

Nit: driver, device, or transport

> +\newcommand{\transportnormative}[3]{#1{Transport Requirements: 
> #2}\label{transportnormative:#3}}
>  \newcommand{\drivernormative}[3]{#1{Driver Requirements: 
> #2}\label{drivernormative:#3}}
>  \newcommand{\devicenormative}[3]{#1{Device Requirements: 
> #2}\label{devicenormative:#3}}
>  \newcounter{clausecounter}
> diff --git a/conformance.tex b/conformance.tex
> index dc00e84e75ae..9bb1c9e2f6ec 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -11,6 +11,10 @@ \section{Conformance Targets}\label{sec:Conformance / 
> Conformance Targets}
>  
>  Conformance targets:
>  \begin{description}
> +\item[Transport] A transport MUST conform to following conformance clauses:

Maybe "MUST conform to one conformance clause"?

> +  \begin{itemize}
> +\item Clause \ref{sec:Conformance / Transport Conformance}.
> +  \end{itemize}
>  \item[Driver] A driver MUST conform to four conformance clauses:
>\begin{itemize}
>  \item Clause \ref{sec:Conformance / Driver Conformance}.

(...)

Apart from the nits above, this looks good to me.

I'd be in favour of initiating a vote for this, after the OASIS
infrastructure migragion has completed and we have dealt with any
fallout from that... that would probably mean March :(

(Oh, and this should probably go to virtio-comment as well.)

[Apologies for not reviewing earlier, but I'm struggling to keep afloat,
and the OASIS infrastructure saga did not exactly help...]


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH] sound: add sampling rates 12000Hz and 24000Hz

[Cornelia Huck]

On Mon, Feb 05 2024, "Pape, Andreas (ADITG/ESS3)"  wrote:

> 24kHz is used for 'super wideband' voice transmission 12kHz is added 'for 
> completeness'
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/184
>
> Signed-off-by: Andreas Pape 
> ---
>  device-types/sound/description.tex | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/device-types/sound/description.tex 
> b/device-types/sound/description.tex
> index 54c9c8e..bad4415 100644
> --- a/device-types/sound/description.tex
> +++ b/device-types/sound/description.tex
> @@ -504,7 +504,9 @@ \subsubsection{PCM Control Messages}\label{sec:Device 
> Types / Sound Device / Dev
>  VIRTIO_SND_PCM_RATE_96000,
>  VIRTIO_SND_PCM_RATE_176400,
>  VIRTIO_SND_PCM_RATE_192000,
> -VIRTIO_SND_PCM_RATE_384000
> +VIRTIO_SND_PCM_RATE_384000,
> +VIRTIO_SND_PCM_RATE_12000,
> +VIRTIO_SND_PCM_RATE_24000
>  };
>  
>  struct virtio_snd_pcm_info {

I think there had been a Reviewed-by: for a previous iteration of this
patch? We can add it while applying (after voting, which I'm going to
initiate); but I'd appreciate it if the previous reviewer could reply
here again.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [RFC PATCH v3] virtio-can: Device specification.

[Cornelia Huck]

On Mon, Jan 15 2024, Cornelia Huck  wrote:

> On Mon, Jan 08 2024, Mikhail Golubev-Ciuchea 
>  wrote:
>
>> Hi all!
>>
>> I kindly request a vote.
>>
>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/186
>
> Oh, that has been a while... I believe that this still fits on top,
> although it has conflicts with virtio-spi, but nothing that a bit of
> fiddling can't sort out.
>
> The main problem is that we're currently in a bit of an unfortunate
> situation regarding voting: There's still a twice-postponed migration of
> the OASIS infrastructure coming up, and I don't want to risk the voting
> period being affected by an outage... hopefully we'll get a new schedule
> soon.
>
> In the meanwhile, if anyone could spare a few cycles looking at this,
> it'd be appreciated. Nothing jumped out at me, but I'm not a CAN expert;
> I'd be happy to open voting but for the looming migration issue.

OK, I'll just go ahead and open a vote, no reason to drag this out
further, and hopefully we're done by the time the migration _really_
happens...


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] [PATCH] Define the DEVICE ID of Virtio Cpu balloon device as 47.

[Cornelia Huck]

On Mon, Jan 29 2024, Shujun Xue  wrote:

> Is there ETA for the migration?

Well, I wish I knew...

>
> Bests,
> Shujun
>
>
> Cornelia Huck 于2024年1月29日 周一上午6:55写道：
>
>> On Tue, Jan 23 2024, Shujun Xue  wrote:
>>
>> > The Virtio CPU balloon device is a primitive device for managing guest
>> > CPU capacity: the device asks for certain CPU cores to be online,
>> > offline or throttled, and the driver performs the requested operation.
>> > This allows the guest to adapt to changes in allowance of underlying
>> > CPU capacity.
>>
>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/187
>>
>> >
>> > Signed-off-by: Shujun Xue 
>> > ---
>> >  content.tex | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/content.tex b/content.tex
>> > index 0a62dce..87f0238 100644
>> > --- a/content.tex
>> > +++ b/content.tex
>> > @@ -739,6 +739,8 @@ \chapter{Device Types}\label{sec:Device Types}
>> >  \hline
>> >  45 &   SPI master \\
>> >  \hline
>> > +47 &   CPU balloon device \\
>> > +\hline
>> >  \end{tabular}
>> >
>> >  Some of the devices above are unspecified by this document,
>>
>> I'm holding off voting for now until the status of the OASIS
>> infrastructure migration is clear (sigh).
>>
>>


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing

[Cornelia Huck]

On Mon, Jan 29 2024, "Michael S. Tsirkin"  wrote:

> On Mon, Jan 29, 2024 at 04:06:01PM +0100, Cornelia Huck wrote:
>> On Wed, Jan 24 2024, "Michael S. Tsirkin"  wrote:
>> 
>> > This is not a huge deal since it's a SHOULD anyway,
>> > so make the new requirement SHOULD too.
>> >
>> > Signed-off-by: Michael S. Tsirkin 
>> > ---
>> >  device-types/net/description.tex | 6 +-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/device-types/net/description.tex 
>> > b/device-types/net/description.tex
>> > index aff5e08..d1d25fe 100644
>> > --- a/device-types/net/description.tex
>> > +++ b/device-types/net/description.tex
>> > @@ -1792,7 +1792,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device 
>> > Types / Network Device / Devi
>> >  
>> >  The device MUST ignore \field{reserved}.
>> >  
>> > -The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and 
>> > VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it was 
>> > not able to change the parameters.
>> > +The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> > +VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if
>> > +it was not able to change coalescing parameters. In this case,
>> 
>> Hm, if we explicitly specify "coalescing parameters" here, should we add
>> it below as well? (We probably should keep the "the".)
>
> maybe "some of the" and in the next sentence "all of the".
>
> so it's atomic: all or nothing.

Makes sense.

>
>> > +the parameters SHOULD remain unchanged, for all VQs.
>> > +
>> >  
>> >  The device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command 
>> > with VIRTIO_NET_ERR if it was not able to change the parameters.

Also here ^ ?


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements

[Cornelia Huck]

On Mon, Jan 29 2024, Viresh Kumar  wrote:

> On 18-12-23, 15:02, Cornelia Huck wrote:
>> Other things that probably should be mandatory:
>> 
>> - A way for the driver to change the device status. Reading it would
>>   probably be a strong SHOULD.
>> - A way to implement config space. (For example, channel devices don't
>>   have a config space or anything similar enough to repurpose, so I had
>>   to invent a mechanism for ccw... maybe future transports will be in
>>   the same boat.)
>
> How about these changes now, before I post them again:
>
> diff --git a/content.tex b/content.tex
> index 0a62dce5f65f..2a86b1041747 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -631,8 +631,81 @@ \section{Device Cleanup}\label{sec:General 
> Initialization And Device Operation /
>  
>  \chapter{Virtio Transport Options}\label{sec:Virtio Transport Options}
>  
> -Virtio can use various different buses, thus the standard is split
> -into virtio general and bus-specific sections.
> +Devices and drivers can use different transport methods to enable
> +interaction, for example PCI, MMIO, or Channel I/O. The transport
> +methods define various aspects of the communication between the device
> +and the driver, like device discovery, exchanging capabilities,
> +interrupt handling, data transfer, etc. For example, in a host/guest
> +architecture, the host might expose a device to the guest on a PCI bus,
> +and the guest will use a PCI-specific driver to interact with it.
> +
> +The standard is split into sections describing general virtio
> +implementation and transport-specific sections.
> +
> +\section{Virtio Transport Requirements}\label{sec:Virtio Transport Options / 
> Virtio Transport Requirements}
> +
> +The transport MUST provide a mechanism for the driver to discover the
> +device.
> +
> +The transport must provide a mechanism for the driver to identify the

s/must/MUST/

> +device type.
> +
> +The transport MUST provide a mechanism for communicating virtqueue
> +configurations between the device and the driver.
> +
> +The transport MUST allow multiple virtqueues per device. The number of
> +virtqueues for a pair of device-driver are governed by the individual
> +device protocol.
> +
> +The transport MUST provide a mechanism that the device and the driver
> +use to access memory for implementing virtqueues.
> +
> +The transport MUST provide a mechanism for the device to notify the
> +driver and a mechanism for the driver to notify the device, for example
> +regarding availability of a buffer on the virtqueue.
> +
> +The transport MAY provide a mechanism for the driver to initiate a
> +reset of the virtqueues and device.
> +
> +The transport MUST provide a mechanism for the driver to read the
> +device status. The transport SHOULD provide a mechanism for the driver to
> +change the device status.

I think the other way around? CCW only gained support for reading the
status with revision 2.

> +
> +The transport MUST provide a mechanism to implement config space between
> +the device and the driver.
> +
> +\devicenormative{\subsection}{Virtio Transport Requirements}{Virtio 
> Transport Options}
> +
> +Any data associated with a device-initiated transaction MUST remain
> +accessible to the driver until the driver acknowledges the transaction
> +to be complete.

Maybe better "The device MUST keep any data (...) accessible to the
driver..." ?

> +
> +The device MUST NOT access the contents of a virtqueue before the
> +driver notifies, in a transport defined way, the device that the
> +virtqueue is ready to be accessed.
> +
> +The device MUST NOT access or modify buffers on a virtqueue after it has
> +notified the driver about their availability.
> +
> +The device MUST reset the virtqueues if requested, in a transport
> +defined way, by the driver.

"in a transport defined way, if the transport provides such a method" ?

> +
> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio 
> Transport Options}
> +
> +The driver MUST acknowledge device notifications, as mandated by the
> +transport.
> +
> +The driver MUST NOT access virtqueue contents before the device notifies
> +about the readiness of the same.
> +
> +The driver MUST NOT access buffers, after it has added them to the
> +virtqueue and notified the device about their availability. The driver
> +MAY access them after the device has processed them and notified the
> +driver of their availability, in a transport defined way.
> +
> +The driver MAY ask the device to reset the virtqueues if, for example,
> +the driver times out waiting for a notification from the device for a
> +previously queued request.
>  
&

[virtio-dev] Re: [virtio] [PATCH] virtio-net: clarify error handling for coalescing

[Cornelia Huck]

On Wed, Jan 24 2024, "Michael S. Tsirkin"  wrote:

> This is not a huge deal since it's a SHOULD anyway,
> so make the new requirement SHOULD too.
>
> Signed-off-by: Michael S. Tsirkin 
> ---
>  device-types/net/description.tex | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/device-types/net/description.tex 
> b/device-types/net/description.tex
> index aff5e08..d1d25fe 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1792,7 +1792,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device 
> Types / Network Device / Devi
>  
>  The device MUST ignore \field{reserved}.
>  
> -The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and 
> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it was not 
> able to change the parameters.
> +The device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> +VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if
> +it was not able to change coalescing parameters. In this case,

Hm, if we explicitly specify "coalescing parameters" here, should we add
it below as well? (We probably should keep the "the".)

> +the parameters SHOULD remain unchanged, for all VQs.
> +
>  
>  The device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command with 
> VIRTIO_NET_ERR if it was not able to change the parameters.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] [PATCH] Define the DEVICE ID of Virtio Cpu balloon device as 47.

[Cornelia Huck]

On Tue, Jan 23 2024, Shujun Xue  wrote:

> The Virtio CPU balloon device is a primitive device for managing guest
> CPU capacity: the device asks for certain CPU cores to be online,
> offline or throttled, and the driver performs the requested operation.
> This allows the guest to adapt to changes in allowance of underlying
> CPU capacity.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/187

>
> Signed-off-by: Shujun Xue 
> ---
>  content.tex | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index 0a62dce..87f0238 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -739,6 +739,8 @@ \chapter{Device Types}\label{sec:Device Types}
>  \hline
>  45 &   SPI master \\
>  \hline
> +47 &   CPU balloon device \\
> +\hline
>  \end{tabular}
>
>  Some of the devices above are unspecified by this document,

I'm holding off voting for now until the status of the OASIS
infrastructure migration is clear (sigh).


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [RFC PATCH v3] virtio-can: Device specification.

[Cornelia Huck]

On Mon, Jan 08 2024, Mikhail Golubev-Ciuchea 
 wrote:

> Hi all!
>
> I kindly request a vote.
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/186

Oh, that has been a while... I believe that this still fits on top,
although it has conflicts with virtio-spi, but nothing that a bit of
fiddling can't sort out.

The main problem is that we're currently in a bit of an unfortunate
situation regarding voting: There's still a twice-postponed migration of
the OASIS infrastructure coming up, and I don't want to risk the voting
period being affected by an outage... hopefully we'll get a new schedule
soon.

In the meanwhile, if anyone could spare a few cycles looking at this,
it'd be appreciated. Nothing jumped out at me, but I'm not a CAN expert;
I'd be happy to open voting but for the looming migration issue.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH v2 1/1] RFC: virtio-bt: add virtio BT device specification

[Cornelia Huck]

On Thu, Dec 07 2023, Igor Skalkin  wrote:

> diff --git a/device-types/bt/description.tex b/device-types/bt/description.tex
> new file mode 100644
> index 000..98311f4
> --- /dev/null
> +++ b/device-types/bt/description.tex
> @@ -0,0 +1,158 @@
> +\section{BT Device}\label{sec:Device Types / BT Device}
> +
> +The virtio-bt device provides an HCI (Host Control Interface) over VirtIO
> +link between the guest HCI device and the host HCI backend.
> +Also, the device can inform the guest driver which operational system / 
> vendor
> + specific HCI extensions it supports.

Is there any good way to describe this without using guest/host
terminology while using the typical guest/host setup as an example
instead?

> +Host Control Interface is described in Bluetooth Core Specification (see
> +\hyperref[intro:bluetooth-core]{Bluetooth Core Specification}).
> +
> +\subsection{Device ID}\label{sec:Device Types / BT Device / Device ID}
> +
> +40
> +
> +\subsection{Virtqueues}\label{sec:Device Types / BT Device / Virtqueues}
> +
> +\begin{description}
> +\item[0] transmitq
> +\item[1] receiveq
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / BT Device / Feature bits}
> +
> +\begin{description}
> +\item[VIRTIO_BT_F_VND_HCI (0)]  The device supports vendor-specific HCI
> +extensions.
> +\item[VIRTIO_BT_F_MSFT_EXT (1)] The device supports Microsoft HCI Extensions.
> +\item[VIRTIO_BT_F_AOSP_EXT (2)] The device supports Android HCI Extensions.
> +\item[VIRTIO_BT_F_CONFIG_V2 (3)] The device uses the second version of the
> +configuration space structure.
> +\end{description}
> +
> +\devicenormative{\subsubsection}{Feature bits}{Device Types / BT Device / 
> Feature bits}
> +
> +The device MAY require the driver to accept the \field{VIRTIO_BT_F_CONFIG_V2}
> +feature bit and use the second version of the configuration layout, because 
> the
> +first one is unaligned, which violates the specification.

ISTR that it was "violates the requirements for configuration space
accesses for some transports"? That might be a bit clearer.

And if that's the case, can we maybe require that the device MUST
require the driver to accept the feature bit if the transport the device
uses cannot deal with v1?

> +
> +The device should offer \field{VIRTIO_BT_F_MSFT_EXT} or

s/should offer/SHOULD offer the/

> +\field{VIRTIO_BT_F_AOSP_EXT} feature bit if it supports correspondingly MSFT 
> or
> +AOSP extension command sets (see \hyperref[intro:bluetooth-aosp]{Bluetooth 
> AOSP
> +HCI Extensions}, \hyperref[intro:bluetooth-msft]{Bluetooth MSFT HCI 
> Extensions}
> +). In case of \field{VIRTIO_BT_F_MSFT_EXT}, device should also set 
> configuration

"the device SHOULD also set the configuration field"

> +field \field{msft_opcode}.
> +
> +The device should offer \field{VIRTIO_BT_F_VND_HCI} feature bit and set

s/should offer/SHOULD offer the/
s/set/set the/

> +configuration field \field{vendor}, if it supports corresponding vendor

s/corresponding/the corresponding/

> +extensions. Also, in case of second configuration version, fields

s/in case of second configuration version/if VIRTIO_BT_F_CONFIG_V2 is 
negotiated/

> +\field{set_bdaddr_opcode} and \field{quirks} can be set according to this

MAY be set? Or SHOULD be set?

> +\field{vendor}.
> +
> +\drivernormative{\subsubsection}{Feature bits}{Device Types / BT Device / 
> Feature bits}
> +
> +The driver MUST accept \field{VIRTIO_BT_F_CONFIG_V2} feature bit if offered 
> by
> +the device.
> +
> +The driver SHOULD accept any of the  \field{VIRTIO_BT_F_VND_HCI},
> +\field{VIRTIO_BT_F_MSFT_EXT} or \field{VIRTIO_BT_F_AOSP_EXT} feature bits if
> +offered by the device.
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / BT Device 
> / Device configuration layout}
> +BT device supports two versions of the configuration structure layout.
> +\subsubsection{Device configuration layout: Version 1}\label{sec:Device 
> Types / BT Device / Device configuration layout / Verion 1}
> +
> +All fields of this configuration are always available and read-only for the
> +driver.
> +\begin{lstlisting}
> +struct virtio_bt_config {
> +u8 type;
> +le16 vendor;
> +le16 msft_opcode;
> +} __attribute__((packed));
> +\end{lstlisting}
> +
> +\begin{description}
> +
> +\item[\field{type}] indicates the type of the device, is it "Primary BR/EDR
> +controller" or "Alternate MAC/PHYs (AMP) secondary controller" and can have 
> the
> +following values:
> +\begin{lstlisting}
> +#define VIRTIO_BT_CONFIG_TYPE_PRIMARY 0
> +#define VIRTIO_BT_CONFIG_TYPE_AMP 1
> +\end{lstlisting}
> +
> +\item[\field{vendor}]
> +Indicates the vendor of the BT device and can have the following mutually
> +exclusive values:
> +\begin{lstlisting}
> +#define VIRTIO_BT_CONFIG_VENDOR_NONE 0
> +#define VIRTIO_BT_CONFIG_VENDOR_ZEPHYR   1
> +#define VIRTIO_BT_CONFIG_VENDOR_INTEL2
> +#define VIRTIO_BT_CONFIG_VENDOR_REALTEK  3
> +\end{lstlisting}
> +The device MUST offer the 

Re: [PATCH] hw/arm/virt: Consolidate valid CPU types

[Cornelia Huck]

On Thu, Jan 11 2024, Gavin Shan  wrote:

> It's found that some of the CPU type names in the array of valid
> CPU types are invalid because their corresponding classes aren't
> registered, as reported by Peter Maydell.
>
> [gshan@gshan build]$ ./qemu-system-arm -machine virt -cpu cortex-a9
> qemu-system-arm: Invalid CPU model: cortex-a9
> The valid models are: cortex-a7, cortex-a15, (null), (null), (null),
> (null), (null), (null), (null), (null), (null), (null), (null), max
>
> Fix it by consolidating the array of valid CPU types. After it's
> applied, we have the following output when TCG is enabled.
>
> [gshan@gshan build]$ ./qemu-system-arm -machine virt -cpu cortex-a9
> qemu-system-arm: Invalid CPU model: cortex-a9
> The valid models are: cortex-a7, cortex-a15, max
>
> [gshan@gshan build]$ ./qemu-system-aarch64 -machine virt -cpu cortex-a9
> qemu-system-aarch64: Invalid CPU model: cortex-a9
> The valid models are: cortex-a7, cortex-a15, cortex-a35, cortex-a55,
> cortex-a72, cortex-a76, cortex-a710, a64fx, neoverse-n1, neoverse-v1,
> neoverse-n2, cortex-a53, cortex-a57, max

Alternatively, we could skip any NULL returns from cpu_model_from_type()
in is_cpu_type_supported(), but I guess leaving out not-provided cpu
types in the first place is cleaner.

>
> Reported-by: Peter Maydell 
> Fixes: fa8c617791 ("hw/arm/virt: Check CPU type in machine_run_board_init()")
> Signed-off-by: Gavin Shan 
> ---
>  hw/arm/virt.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [PATCH v4 4/4] hw/misc/pvpanic: add support for normal shutdowns

[Cornelia Huck]

On Sun, Jan 07 2024, Thomas Weißschuh  wrote:

> Shutdown requests are normally hardware dependent.
> By extending pvpanic to also handle shutdown requests, guests can
> submit such requests with an easily implementable and cross-platform
> mechanism.
>
> Signed-off-by: Thomas Weißschuh 
> ---
>  docs/specs/pvpanic.rst| 2 ++
>  hw/misc/pvpanic.c | 5 +
>  include/hw/misc/pvpanic.h | 3 ++-
>  3 files changed, 9 insertions(+), 1 deletion(-)

Not deeply involved with this, but looks reasonable to me.

Acked-by: Cornelia Huck 

Re: [PATCH v4 3/4] tests/qtest/pvpanic: use centralized definition of supported events

[Cornelia Huck]

On Sun, Jan 07 2024, Thomas Weißschuh  wrote:

> Avoid the necessity to update all tests when new events are added
> to the device.
>
> Acked-by: Thomas Huth 
> Signed-off-by: Thomas Weißschuh 
> ---
>  tests/qtest/pvpanic-pci-test.c | 5 +++--
>  tests/qtest/pvpanic-test.c | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [PATCH v4 2/4] hw/misc/pvpanic: centralize definition of supported events

[Cornelia Huck]

On Sun, Jan 07 2024, Thomas Weißschuh  wrote:

> The different components of pvpanic duplicate the list of supported
> events. Move it to the shared header file to minimize changes when new
> events are added.
>
> Signed-off-by: Thomas Weißschuh 
> ---
>  hw/misc/pvpanic-isa.c | 2 +-
>  hw/misc/pvpanic-pci.c | 2 +-
>  hw/misc/pvpanic.c | 2 +-
>  include/hw/misc/pvpanic.h | 1 +
>  4 files changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [PATCH v4 1/4] linux-headers: drop pvpanic.h

[Cornelia Huck]

On Sun, Jan 07 2024, Thomas Weißschuh  wrote:

> misc/pvpanic.h from the Linux UAPI does not define a Linux UAPI but a
> qemu device API.
>
> This leads to a weird process when updates to the interface are needed:
> 1) Change to the specification in the qemu tree
> 2) Change to the header in the Linux tree
> 3) Re-import of the header into Qemu.
>
> The kernel prefers to drop the header anyways.
>
> Prepare for the removal from the Linux UAPI headers by moving the
> contents to the existing pvpanic.h header.
>
> Link: https://lore.kernel.org/lkml/2023110431-pacemaker-pruning-0e4c@gregkh/
> Signed-off-by: Thomas Weißschuh 
> ---
>  hw/misc/pvpanic-isa.c| 1 -
>  hw/misc/pvpanic-pci.c| 1 -
>  hw/misc/pvpanic.c| 1 -
>  include/hw/misc/pvpanic.h| 3 +++
>  include/standard-headers/linux/pvpanic.h | 9 -
>  scripts/update-linux-headers.sh  | 3 +--
>  6 files changed, 4 insertions(+), 14 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [virtio-dev] [PATCH v8] virtio-net: correct conditions for devices to validate packet checksum

[Cornelia Huck]

On Thu, Jan 11 2024, Heng Qi  wrote:

> 在 2024/1/4 下午12:47, Heng Qi 写道:
>>
>>
>> 在 2024/1/3 下午2:35, Heng Qi 写道:
>>> There is a historical error in virtio spec:
>>>    "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device *MUST* 
>>> set flags
>>>     to zero and SHOULD supply a fully checksummed packet to the driver."
>>>
>>> Currently in Linux and virtio-related implementations, the device 
>>> validates
>>> the packet checksum and sets DATA_VALID regardless of whether
>>> VIRTIO_NET_F_GUEST_CSUM is negotiated.
>>>
>>> Please refer to the following summary and thread[1] for details and 
>>> reasons:
>>>
>>> Summary
>>> =
>>> 1. GUEST_CSUM at virtio spec 0.95 is intended to be compatible with 
>>> partially
>>> checksummed packets (NEEDS_CSUM <-> CHECKSUM_PARTIAL). So GUEST_CSUM 
>>> is mapped
>>> to NETIF_F_RXCSUM.
>>> GUEST_CSUM only indicates whether the driver handles partially 
>>> checksummed packets.
>>> When XDP is loaded, the GUEST_CSUM's offload will be disabled, which 
>>> means that
>>> packets have NEEDS_CSUM set will be dropped, and packets have 
>>> DATA_VALID set will
>>> still be received.
>>>
>>> 2. When DATA_VALID was added to Linux in 2011[2] and virtio1.0, it 
>>> was actually
>>> expected that rx checksum offload (the driver can set 
>>> CHECKSUM_UNNECESSARY) had
>>> nothing to do with whether GUEST_CSUM was negotiated. But due to an 
>>> error, below
>>> desctiption was added incorrectly:
>>>    "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device *MUST* 
>>> set flags
>>>     to zero and SHOULD supply a fully checksummed packet to the driver."
>>>
>>> 3. We now hope to correct this error. Let the setting of DATA_VALID 
>>> not be
>>> controlled by whether GUEST_CSUM is negotiated, but only controlled 
>>> by whether
>>> rx checksum offload is enabled on the OS side. The state of this rx 
>>> checksum
>>> offload is not aware of the device.
>>>
>>> 4. [Optional] NETIF_F_RXCSUM corresponding to rx checksum offload 
>>> should be
>>> added to dev->hw_features. When the user turns off rx checksum 
>>> offload through
>>> ethtool -K, neither NEEDS_CSUM nor DATA_VALID should be taken care 
>>> of, that is,
>>> all packets will be marked as CHECKSUM_NONE.
>>>
>>> Related Link
>>> =
>>> [1] 
>>> https://lists.oasis-open.org/archives/virtio-comment/202312/msg00135.html
>>> [2] 10a8d94a9574 ("virtio_net: introduce VIRTIO_NET_HDR_F_DATA_VALID")
>>>
>>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/185
>>
>> Hi Michael, Cornelia.
>>
>> Could you please open a voting for this fix?
>
> Hi Michael and Cornelia.
>
> Sincerely asking if you missed this message?

I've been back from end-of-year PTO only this week...

I'm currently waiting for a confirmed updated schedule for the
infrastructure migration, as I don't want to risk having the platform
out of order in the middle of the voting period.

>
> In addition, Jason recently responded with his reviewed-by tag.
> Do I need to publish a new version carrying his tag (thanks Jason)?

No, we can add his R-b when pushing to git.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev][PATCH V9 2/2] virtio-spi: add the device specification

[Cornelia Huck]

On Tue, Dec 12 2023, Haixu Cui  wrote:

> The Virtio SPI (Serial Peripheral Interface) device is a virtual
> SPI controller that allows the driver to operate and use the SPI
> controller under the control of the host.
>
> This patch adds the specification for virtio-spi.
>
> Signed-off-by: Haixu Cui 
> Reviewed-by: Viresh Kumar 
> ---
>  device-types/spi/description.tex| 281 
>  device-types/spi/device-conformance.tex |   7 +
>  device-types/spi/driver-conformance.tex |   7 +
>  3 files changed, 295 insertions(+)
>  create mode 100644 device-types/spi/description.tex
>  create mode 100644 device-types/spi/device-conformance.tex
>  create mode 100644 device-types/spi/driver-conformance.tex

Oh, and I just noticed that you also need to include
{device,driver}-conformance.tex in conformance.tex.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev][PATCH V9 2/2] virtio-spi: add the device specification

[Cornelia Huck]

On Thu, Dec 21 2023, Haixu Cui  wrote:

> Hi Cornelia,
>
> On 12/21/2023 4:44 PM, Cornelia Huck wrote:
>> On Thu, Dec 21 2023, Haixu Cui  wrote:
>>>> Please move inclusion of this new file into content.tex here, instead of
>>>> including a not-yet-existing file in the previous patch.
>>>>
>>>> (...)
>>>
>>>
>>> Sorry, I don't quite understand here. Maybe I should not add the
>>> following line in content.tex, is that so?
>>>
>>> \input{device-types/spi/description.tex}
>> 
>> No, please add this line; but do so in this patch instead of the
>> previous one that only changes the wording.
>
> I am not clear where to put this line? In the commit message or in the 
> main text, e.g. at the start of the description.tex? As follows:
>
> diff --git a/device-types/spi/description.tex 
> b/device-types/spi/description.tex
> new file mode 100644
> index 000..b76258c
> --- /dev/null
> +++ b/device-types/spi/description.tex
> @@ -0,0 +1,281 @@
> +\input{device-types/spi/description.tex}
> +\section{SPI Controller Device}\label{sec:Device Types / SPI Controller 
> Device}
> +
> ...

Ah no, the inclusion is of course in the right place where you put it in
context.tex; the *patch hunk* should be moved from patch 1 to patch 2,
that's all. Hope that's more clear now :)


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev][PATCH V9 2/2] virtio-spi: add the device specification

[Cornelia Huck]

On Thu, Dec 21 2023, Haixu Cui  wrote:

> Hi Cornelia,
>  Much appreciated for your comments and please refer to my response.
>
> On 12/18/2023 7:04 PM, Cornelia Huck wrote:
>> On Tue, Dec 12 2023, Haixu Cui  wrote:
>> 
>> [BTW: A changelog would be useful, either in the patch or in the cover
>> letter.]
>
> Sure, I summarize the major changes between these versions and will add 
> in next patch:
>
> changelog:
> v8->v9:
> - add explanation of bits_per_word_mask in config space
> v7->v8:
> - change device to host
> v6->v7:
> - fix the format problem, syntax problem
> v5->v6:
> - use driver/device instead guest/host
> - add the definition of some terminologies
> - use controller instead of master throughout the spec
> - add buffer length validation for full-duplex transfer
> v4->v5:
> - use controller instead of master
> - fix indentation issue
> - extend the config space to expose the backend supported features
> - add another result value to indicate parameter error
> - add device and driver requirement about parameter checkig
> v3->v4:
> - fix the spell error
> - bus_num is not SOC-specific, remove it
> - add driver requirement to deal with the situation that the cs delay
> parameters are not 0 but the backend doesn't support cs timing setting
> v2->v3
> - remove unnecessary statements and driver implimentation details
> - add the parameters about cs timing delay and transfer delay
> - use "le32" instead of "u32"
> - swap the rx_buf and tx_buf in the request format
> - add the parameters about transfer bit width
> v1->v2:
> - explain SPI when it is firstly used
> - update the ambiguous expression of virtqueue
> v0->v1:
> - add definition of abbreviation SPI
> - remove the ID

Thanks. Might be best to add this in the cover letter.

> ---
>
>> 
>>> The Virtio SPI (Serial Peripheral Interface) device is a virtual
>>> SPI controller that allows the driver to operate and use the SPI
>>> controller under the control of the host.
>>>
>>> This patch adds the specification for virtio-spi.
>>>
>>> Signed-off-by: Haixu Cui 
>>> Reviewed-by: Viresh Kumar 
>>> ---
>>>   device-types/spi/description.tex| 281 
>>>   device-types/spi/device-conformance.tex |   7 +
>>>   device-types/spi/driver-conformance.tex |   7 +
>>>   3 files changed, 295 insertions(+)
>>>   create mode 100644 device-types/spi/description.tex
>>>   create mode 100644 device-types/spi/device-conformance.tex
>>>   create mode 100644 device-types/spi/driver-conformance.tex
>>>
>>> diff --git a/device-types/spi/description.tex 
>>> b/device-types/spi/description.tex
>> 
>> Please move inclusion of this new file into content.tex here, instead of
>> including a not-yet-existing file in the previous patch.
>> 
>> (...)
>
>
> Sorry, I don't quite understand here. Maybe I should not add the 
> following line in content.tex, is that so?
>
> \input{device-types/spi/description.tex}

No, please add this line; but do so in this patch instead of the
previous one that only changes the wording.

>
>> 
>>> +\field{mode_func_supported} indicates whether the following features are 
>>> supported or not:
>>> +bit 0-1: CPHA feature,
>>> +0b00: invalid, must support as least one CPHA setting.
>>> +0b01: supports CPHA=0 only;
>>> +0b10: supports CPHA=1 only;
>>> +0b11: supports CPHA=0 and CPHA=1;
>>> +
>>> +bit 2-3: CPOL feature,
>>> +0b00: invalid, must support as least one CPOL setting.
>>> +0b01: supports CPOL=0 only;
>>> +0b10: supports CPOL=1 only;
>>> +0b11: supports CPOL=0 and CPOL=1;
>>> +
>>> +bit 4: chipselect active high feature, 0 for unsupported and 1 for 
>>> supported,
>>> +chipselect active low must always be supported.
>>> +
>>> +bit 5: LSB first feature, 0 for unsupported and 1 for supported, 
>>> MSB first must always be
>>> +supported.
>>> +
>>> +bit 6: loopback mode feature, 0 for unsupported and 1 for 
>>> supported, normal mode
>>> +must always be supported.
>>> +
>>> +Note: CPOL is clock polarity and CPHA is clock phase. If CPOL is 0, the 
>>> clock idles at the logical
>>> +low voltage, otherwise it idles at the logical high voltage. CPHA 
>>>

[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements

[Cornelia Huck]

On Mon, Dec 18 2023, Bill Mills  wrote:

> On 12/18/23 9:19 AM, Alex Bennée wrote:
>> Cornelia Huck  writes:
>> 
>>> On Wed, Dec 06 2023, Viresh Kumar  wrote:
>>>
>>>> On 05-12-23, 14:18, Cornelia Huck wrote:
>>>>> On Tue, Dec 05 2023, Viresh Kumar  wrote:
>> 
>>>>
>>>> - The transport MUST provide a mechanism for the device and the driver
>>>>to notify each other, for example about availability of a buffer on
>>>>the virtqueue.
>>>
>>> Probably a mechanism for the device to notify the driver, and a
>>> mechanism for the driver to notify the device? They can be different, as
>>> long both of them are present.
>>>
>>>>
>>>> - The transport SHOULD provide a mechanism for the driver to initiate
>>>>a reset of the virtqueues and device.
>>>
>>> Can we mandate a mechanism for a device reset? Reset of virtqueues needs
>>> to be optional, I'm not sure whether a SHOULD would be appropriate for
>>> that (or maybe we should finally come up with something for ccw ;)
>>>
>>> Other things that probably should be mandatory:
>>>
>>> - A way for the driver to change the device status. Reading it would
>>>probably be a strong SHOULD.
>>> - A way to implement config space. (For example, channel devices don't
>>>have a config space or anything similar enough to repurpose, so I had
>>>to invent a mechanism for ccw... maybe future transports will be in
>>>the same boat.)
>> 
>> I think Bill has run into problems with config space on OpenAMP setups
>> where there can be no specific trapping between domains making it hard
>> to manage a config space where both sides might want to make updates. I
>> guess the original MMIO transport gets away with it because config space
>> is always in the trap-and-emulate address space in a normal VM/emulation
>> situation.
>> 
>> Bill,
>> 
>> Is the plan to introduce a new transport that manages config in a
>> different way?
>> 
>
> I/We had a couple of ideas.  This is a thorny issue if you want 
> something that 100% matches the implications of the virtio spec.  If you 
> want something that works with existing virtio protocols it is not as 
> bad but still tricky.
>
> If you read the virtio spec it implies you could have a device with zero 
> virtqueues and just does things with the config space.  To me this 
> should be a SHOULD NOT.  Something like "The config space SHOULD NOT be 
> used as a mechanism for frequent changes."
>
> To me the config space is best used as a device to driver info space 
> with infrequent updates from both sides.  I don't know how that should 
> be expressed in the spec.

We do have some text suggesting that this should work like that, i.e. in
"Config Space" we say "Device configuration space is generally used for
rarely-changing or initialization-time parameters." and in Appendix B we
say "Device configuration space should only be used for
initialization-time parameters. It is a limited resource with no
synchronization between field written by the driver, so for most uses it
is better to use a virtqueue to update configuration information". I'm
not sure where to add a normative statement, given that this is
something that device types need to take care of.

>
> It is true that virtio-pci and virtio-mmio can signal changes in config 
> space from the driver to the device.  However the device driver can read 
> the config space asynchronously whenever it wants.  The generation 
> counter was added to fix non-atomic updates from the device.
>
> The generation counter in virtio-mmio and virtio-pci transport config 
> spaces only work because the hypervisor can see each read attempt to the 
> config space.  The virtio-pci suggests not incrementing the generation 
> counter unless the guest is doing a read.  (Because the generation 
> counter on virtio-pci is small.)
>
> In addition, config updates from the driver at not formally signaled and 
> it again relies on the hypervisor intersecting writes to the config space.

The configuration space for ccw is really different conceptually: both
reading from and writing to it are initiated by the driver sending
commands, and the architecture makes sure that reads and writes are
processed by the device strictly in the order that the driver sent
them. IOW, unless the device messes up, the driver will get consistent
information.

>
> Anyway I have thought of two solutions for the config space that do not 
> rely on hypervisor magic.
>
> 1) Config generation counter odd == up

Re: [virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements

[Cornelia Huck]

On Mon, Dec 18 2023, Alex Bennée  wrote:

> Cornelia Huck  writes:
>
>> On Tue, Dec 05 2023, Alex Bennée  wrote:
>>
>>> Cornelia Huck  writes:
>>>
>>>> On Tue, Dec 05 2023, Viresh Kumar  wrote:
>>>>> +
>>>>> +The device MUST present each event, in a transport defined way, from the
>>>>> +moment it takes place until the driver acknowledges the event.
>>>>
> 
>>>>> +
>>>>> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio 
>>>>> Transport Options}
>>>>> +
>>>>> +The driver MUST NOT access guest memory locations outside what's made
>>>>> +available by the device to the driver.
>>>>
>>>> I don't think that makes sense -- I'd assume most guest memory locations
>>>> do not have anything to do with virtio, and we should try to avoid
>>>> host/guest terminology.
>>>
>>> I agree guest memory isn't the right terminology here. However there are
>>> discussions about how to implement secure buffers for VirtIO - so for
>>> example a buffer mediated by some sort of secure layer. In those cases
>>> the driver may not have access to it outside of the transactions. 
>>
>> Yes, I think we need to limit the scope of "guest memory" here. I think
>> we are basically wanting to deal with any memory used by virtio (device
>> type including memory access controlled by it, transport, and the
>> protocol itself). We would be talking about memory made available to the
>> device by the driver for explicit usage to implement the virtio spec. I
>> think this would cover mediation by a secure layer as well (with the
>> driver calling into that secure layer?) Or does the (host) device end up
>> donating memory to the (guest) driver, and we need to make sure it
>> doesn't scribble over it?
>
> I'm not sure if we have example of the host donating memory apart from
> the sort of static partitioning we see with guests on start-up where a
> region is defined as shared. The Xen grant model leaves the guest to
> grant access to its own pages to the backend.
>
> I guess for firmware mediated sharing this would still be driven by the
> guest rather than the host?

Yes, I think the guest telling the firmware which parts of its memory
the host may access is the usual pattern, or at least what I've seen so
far.

>>
>>>>> +
>>>>> +The driver MUST NOT access virtqueue contents before the device notifies
>>>>> +about the readiness of the same.
>>>>> +
>>>>> +The driver MUST NOT access buffers, after it has added them to the
>>>>> +virtqueue and notified the device about their availability. The driver
>>>>> +MAY access them after the device has processed them and notified the
>>>>> +driver of their availability, in a transport defined way.
>>>>> +
>>>>> +The driver MAY ask the device to reset the virtqueues if, for example,
>>>>> +the driver times out waiting for a notification from the device for a
>>>>> +previously queued request.
>>>>
>>>> Again, I believe this has already been covered in the generic
>>>> sections -- do we instead need to specify that a transport MUST provide
>>>> a method to do xy? (or SHOULD, MAY, as applicable -- it would be good to
>>>> list explicitly what is mandatory for a transport to implement, and what
>>>> is optional.)
>>>
>>> Yes I think so. The s390x channel transport gets referenced because it
>>> has a nice enumerated list of operations. It would be good to codify
>>> which operations are mandatory for all transports and which are
>>> optional.
>>
>> The problem with the ccw transport is that while it has a nice list of
>> operations, (a) it only covers guest-initiated actions,
>
> What examples of host initiated actions are there (aside from an IPI
> indicating a receive VirtQueue has buffers waiting)?

Also notifications for configuration changes; but I think we can put all
of this under the device->driver notification header. (Hmm, we probably
should change all those host/guest references for ccw some day...)

>
>> (b) probably not
>> all of them shold be mandatory (and some of them are more of an artifact
>> of how channel I/O works),
>
> These ones?
>
>   #define CCW_CMD_SET_IND 0x43
>   #define CCW_CMD_SET_CONF_IND 0x53
>   #define CCW_CMD_SET_IND_ADAPTER 0x73

Yes, and also CCW_CMD_SET_VIRTIO_REV (I think there used to be some
interest to implement something like that for mmio, but I don't think
anything ever ended up being specified?)

>
>> and (c) it only implements a subset of the
>> defined operations (which makes the not-implemented ones de facto
>> optional, of course :) But yes, we could use it as a starting point.
>
> Got to start somewhere :-)

Indeed :)


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements

[Cornelia Huck]

On Wed, Dec 06 2023, Viresh Kumar  wrote:

> On 05-12-23, 14:18, Cornelia Huck wrote:
>> On Tue, Dec 05 2023, Viresh Kumar  wrote:
>> > +\section{Virtio Transport Requirements}\label{sec:Virtio Transport 
>> > Options / Virtio Transport Requirements}
>> > +
>> > +\devicenormative{\subsection}{Virtio Transport Requirements}{Virtio 
>> > Transport Options}
>> 
>> I'm not sure we can introduce MUST (NOT) requirements for basic
>> functionality after the spec has been published for quite a time already
>> (although I'd assume every implementation is fulfilling the requirements
>> anyway)... thoughts?
>
> Fair point. I think it would be okay to use MUST (NOT) if we are sure
> that the existing implementations are fulfilling the requirements.
> Else maybe we can use SHOULD (NOT) ?

Yes, I think if we are just spelling out things explicitly in one place
that have been required anyway (by existing transports etc.), we are
fine using MUST; everything where someone could have chosen a different
implementation needs to be SHOULD.

(...)

>> > +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio 
>> > Transport Options}
>> > +
>> > +The driver MUST NOT access guest memory locations outside what's made
>> > +available by the device to the driver.
>> 
>> I don't think that makes sense -- I'd assume most guest memory locations
>> do not have anything to do with virtio, and we should try to avoid
>> host/guest terminology.
>
> Hmm, you are right about the guest/host terminology. This comes mostly
> from the MMIO transport section, where a guest presents 0x100 bytes of
> memory, followed by configuration space (whose length is device/driver
> dependent). The MMIO section has this:
>
> "The driver MUST NOT access memory locations not described in the
> table \ref{tab:Virtio Transport Options / Virtio Over MMIO / MMIO Device 
> Register Layout}
> (or, in case of the configuration space, described in the device 
> specification),
> MUST NOT write to the read-only registers (direction R) and
> MUST NOT read from the write-only registers (direction W)."

Thanks for clarifying where this came from! See my suggestions in the
other fork of this thread.

>
>> > +The driver MUST NOT write to the read-only memory area and MUST NOT read
>> > +from the write-only memory area.
>> 
>> Which memory areas does that refer to? Parts of the transport-specific
>> data structures?
>
> Yes, based again on the above MMIO section, but perhaps apply to other
> transports as well..

I think we can't assume transports to use r/o and w/o memory areas in
all cases -- for example, ccw is largely based on passing buffers
around. Of course, if you look at the actual structures, there are
always fields that are to be used for reading or writing only.

>
>> > +The driver MUST acknowledge events presented by the device, as mandated
>> > +by the transport.
>> 
>> I don't think this is quite correct in the absolute -- for example, it
>> should be fine to not acknowledge events if some overriding event comes
>> along, or if the driver initiates a reset.
>
> What about:
>
> The driver SHOULD acknowledge events presented by the device, as
> mandated by the transport, unless a new event from the device
> overrides the previous one or the driver initiates a reset.

Should we do s/events/notifications/, or do we also cover transactions
initiated by the device explicitly (I'd assume that the device would
notify the driver im- or explicitly if it has to do something?)

>
>> > +The driver MUST NOT access virtqueue contents before the device notifies
>> > +about the readiness of the same.
>> > +
>> > +The driver MUST NOT access buffers, after it has added them to the
>> > +virtqueue and notified the device about their availability. The driver
>> > +MAY access them after the device has processed them and notified the
>> > +driver of their availability, in a transport defined way.
>> > +
>> > +The driver MAY ask the device to reset the virtqueues if, for example,
>> > +the driver times out waiting for a notification from the device for a
>> > +previously queued request.
>> 
>> Again, I believe this has already been covered in the generic
>> sections
>
> Right, but having it all at a single place makes it more convenient,
> instead of looking at implementations.

In that case, I'm fine for the first two; however, I'm not sure what the
third one adds here -- I don't think we ever said anything about when
the driver can initiate a reset, it can basically be at any time, and it
overrides whatever the de

[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements

[Cornelia Huck]

On Tue, Dec 05 2023, Alex Bennée  wrote:

> Cornelia Huck  writes:
>
>> On Tue, Dec 05 2023, Viresh Kumar  wrote:
>>> +
>>> +The device MUST present each event, in a transport defined way, from the
>>> +moment it takes place until the driver acknowledges the event.
>>
>> I don't believe "event" is well-defined here.
>
> Maybe:
>
> "A device initiated transaction can isn't considered complete until
> acknowledged by the driver. As such data MUST remain visible to the
> driver until the transaction is complete"?

Transaction is good, what about:

"Any data associated with a device-initiated transaction MUST remain
accessible to the driver until the driver acknowledges the transaction
to be complete."

>
>>
>>> +
>>> +The device MUST NOT access virtqueue's contents before the driver
>>> +notifies that the queue is ready for access, in a transport defined way.
>>> +
>>> +The device MUST NOT access buffers on the virtqueue, after it has
>>> +modified them and notified the driver about their availability.
>>> +
>>> +The device MUST reset the virtqueues if requested by the driver, in a
>>> +transport defined way.
>>
>> Isn't all of this already defined in one place of the spec or another?
>
> I think the recent example is the virtio-sound driver continuing to feed
> data into buffers after those buffers where submitted into the
> virtqueue. We should be explicit that the only time both sides of a
> VirtIO implementation can access things at the same time is with
> explicitly shared memory (and you need some sort of mechanism to mediate
> that to avoid chaos).

Fair enough, let's make it explicit if people already stumbled
here. Some rewording suggestions:

"The device MUST NOT access the contents of a virtqueue before the
driver notifies, in a transport defined way, the device that the
virtqueue is ready to be accessed.

The device MUST NOT access or modify buffers on a virtqueue after it has
notified the driver about their availability.

The device MUST reset the virtqueues if requested, in a transport
defined way, by the driver."

>
>>> +
>>> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio 
>>> Transport Options}
>>> +
>>> +The driver MUST NOT access guest memory locations outside what's made
>>> +available by the device to the driver.
>>
>> I don't think that makes sense -- I'd assume most guest memory locations
>> do not have anything to do with virtio, and we should try to avoid
>> host/guest terminology.
>
> I agree guest memory isn't the right terminology here. However there are
> discussions about how to implement secure buffers for VirtIO - so for
> example a buffer mediated by some sort of secure layer. In those cases
> the driver may not have access to it outside of the transactions. 

Yes, I think we need to limit the scope of "guest memory" here. I think
we are basically wanting to deal with any memory used by virtio (device
type including memory access controlled by it, transport, and the
protocol itself). We would be talking about memory made available to the
device by the driver for explicit usage to implement the virtio spec. I
think this would cover mediation by a secure layer as well (with the
driver calling into that secure layer?) Or does the (host) device end up
donating memory to the (guest) driver, and we need to make sure it
doesn't scribble over it?

>>> +
>>> +The driver MUST NOT access virtqueue contents before the device notifies
>>> +about the readiness of the same.
>>> +
>>> +The driver MUST NOT access buffers, after it has added them to the
>>> +virtqueue and notified the device about their availability. The driver
>>> +MAY access them after the device has processed them and notified the
>>> +driver of their availability, in a transport defined way.
>>> +
>>> +The driver MAY ask the device to reset the virtqueues if, for example,
>>> +the driver times out waiting for a notification from the device for a
>>> +previously queued request.
>>
>> Again, I believe this has already been covered in the generic
>> sections -- do we instead need to specify that a transport MUST provide
>> a method to do xy? (or SHOULD, MAY, as applicable -- it would be good to
>> list explicitly what is mandatory for a transport to implement, and what
>> is optional.)
>
> Yes I think so. The s390x channel transport gets referenced because it
> has a nice enumerated list of operations. It would be good to codify
> which operations are mandatory for all transports and which are
> optional.

The problem

Re: [virtio-dev][PATCH V9 2/2] virtio-spi: add the device specification

[Cornelia Huck]

On Tue, Dec 12 2023, Haixu Cui  wrote:

[BTW: A changelog would be useful, either in the patch or in the cover
letter.]

> The Virtio SPI (Serial Peripheral Interface) device is a virtual
> SPI controller that allows the driver to operate and use the SPI
> controller under the control of the host.
>
> This patch adds the specification for virtio-spi.
>
> Signed-off-by: Haixu Cui 
> Reviewed-by: Viresh Kumar 
> ---
>  device-types/spi/description.tex| 281 
>  device-types/spi/device-conformance.tex |   7 +
>  device-types/spi/driver-conformance.tex |   7 +
>  3 files changed, 295 insertions(+)
>  create mode 100644 device-types/spi/description.tex
>  create mode 100644 device-types/spi/device-conformance.tex
>  create mode 100644 device-types/spi/driver-conformance.tex
>
> diff --git a/device-types/spi/description.tex 
> b/device-types/spi/description.tex

Please move inclusion of this new file into content.tex here, instead of
including a not-yet-existing file in the previous patch.

(...)

> +\field{mode_func_supported} indicates whether the following features are 
> supported or not:
> +bit 0-1: CPHA feature,
> +0b00: invalid, must support as least one CPHA setting.
> +0b01: supports CPHA=0 only;
> +0b10: supports CPHA=1 only;
> +0b11: supports CPHA=0 and CPHA=1;
> +
> +bit 2-3: CPOL feature,
> +0b00: invalid, must support as least one CPOL setting.
> +0b01: supports CPOL=0 only;
> +0b10: supports CPOL=1 only;
> +0b11: supports CPOL=0 and CPOL=1;
> +
> +bit 4: chipselect active high feature, 0 for unsupported and 1 for 
> supported,
> +chipselect active low must always be supported.
> +
> +bit 5: LSB first feature, 0 for unsupported and 1 for supported, MSB 
> first must always be
> +supported.
> +
> +bit 6: loopback mode feature, 0 for unsupported and 1 for supported, 
> normal mode
> +must always be supported.
> +
> +Note: CPOL is clock polarity and CPHA is clock phase. If CPOL is 0, the 
> clock idles at the logical
> +low voltage, otherwise it idles at the logical high voltage. CPHA determines 
> how data is outputted
> +and sampled.

Shouldn't you also specify what CPHA==0 and CPHA==1 mean?


(...)

> +VIRTIO_SPI_TRANS_ERR indicates a transfer error, which means that the 
> parameters are all
> +valid but the trasnfer process failed.

s/trasnfer/transfer/

LGTM otherwise. Does anybody else have comments?


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] Changes coming to this mail list system

[Cornelia Huck]

On Tue, Dec 05 2023, Scott McGrath  wrote:

[dropped non-virtio lists, added the other virtio lists]

> The Transition Schedule
>
> Starting 14 December, there may be as much as several days of system
> downtime. During the downtime you will not receive email messages sent to
> this list or have access to the archive.
>
> We will announce the timing specific as we get closer to the transition.

I've received off-list notification that this change is now postponed to
January -- can you please confirm? It is important for us to plan how we
are going to proceed with the 1.3 version of the virtio spec.  (And it
is of course also important for people reading those lists, who may not
have an account in the system.)


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] virtio 1.3: update

[Cornelia Huck]

You may have been wondering what the current state of virtio 1.3 is; the
tl;dr is: we have to deal with some unfortunate delays, but it is really
nearing release now.

The current state
-

virtio-1.3-csd01 (committee specification draft) has been out for public
review, and that 30 day period has just finished, with no additional
comments being received. This means we're ready to start the process to
get the committee specification draft released as an actual committee
specification.

The current issue
-

Moving from CSD to CS requires that the TC votes on it. Unfortunately,
this will not be possible to fit into the remaining time before the
OASIS infrastructure downtime due to the upcoming migration. While we
hope that the new infrastructure will be in place quickly, it is
unlikely that we will be able to do a vote before many/most of the
voting members will be away for end-of-year holidays. [We actually need
*two* votes in the end, with each ballot open for seven days.]

The current plan


We'll do any preparation work (which should be really minor) now, and
start the voting process early in January. I'm reasonably confident
that, once triggered, we should have finished the process in early
February the latest.

Apologies again for the numerous delays the spec encountered along the
way, it's been pretty frustrating.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] [PATCH V6 2/2] virtio-spi: add the device specification

[Cornelia Huck]

On Mon, Dec 04 2023, Haixu Cui  wrote:

> Hi Huck,
>
> On 12/1/2023 7:01 PM, Cornelia Huck wrote:
>> On Fri, Dec 01 2023, Haixu Cui  wrote:
>> 
>>>>> +Note: chipselect is an electrical signal controlled by the SPI 
>>>>> controller, used to select the
>>>>> +SPI slaves connected to the controller.
>>>>
>>>> I wonder whether another term is now more commonly used... the Wikipedia
>>>> article uses main/sub, is there a term that is usually used together
>>>> with "controller"?
>>>
>>> In Wikipedia,
>>>
>>> "Historically, this arrangement has been called master/slave. But to
>>> avoid referencing slavery, alternative names discussed in § Alternative
>>> namings have been used."
>>>
>>> I am not quite sure if main/sub is widely used, but in latest Linux,
>>> still use master/slave but master is gradually replaced by "controller".
>>>
>>> So I think "controller" here does make sense. What is your suggestion?
>> 
>> "controller" is fine, I'm just wondering if "slave" has started to be
>> replaced with something else as well... if it hasn't, we probably need
>> to stick with it to avoid confusing readers.
>> 
>
> slave is still being used, but it's better to be used along with "master".
>
> Here I prefer using "SPI peripherals" instead of "SPI slaves". Looking 
> forward to your advice. Thank you so much.

"SPI peripherals" sounds good to me.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH] virtio-transport: Clarify requirements

[Cornelia Huck]

On Tue, Dec 05 2023, Viresh Kumar  wrote:

> The virtio documentation currently doesn't define any generic
> requirements that are applicable to all transports. They can be useful
> while adding support for a new transport.
>
> This commit tries to define the same.

Thank you for tackling this, albeit the devil's in the details :)

>
> Signed-off-by: Viresh Kumar 
> ---
>  content.tex | 48 ++--
>  1 file changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index 0a62dce5f65f..d4d5e7d7045b 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -631,8 +631,52 @@ \section{Device Cleanup}\label{sec:General 
> Initialization And Device Operation /
>  
>  \chapter{Virtio Transport Options}\label{sec:Virtio Transport Options}
>  
> -Virtio can use various different buses, thus the standard is split
> -into virtio general and bus-specific sections.
> +The virtio devices are exposed to the guest as if they are physical
> +devices using a specific transport method, like PCI, MMIO or Channel
> +I/O.

I'm not sure we can talk about "exposed to the guest" here, except as an
example... maybe if we reword the whole paragraph (see my suggestion
below.)

> The transport methods define various aspects of the communication
> +between the device and the driver, like device discovery, exchanging
> +capabilities, interrupt handling, data transfer, etc.. Virtio can use
> +various different buses, thus the standard is split into virtio general
> +and bus-specific sections.

I think we should concentrate on the transport being what links device
and driver together... what about (reusing parts of your writeup):

"Devices and drivers can use different transport methods to enable
interaction, for example PCI, MMIO, or Channel I/O. The transport
methods define various aspects of the communication between the device
and the driver, like device discovery, exchanging capabilities,
interrupt handling, data transfer, etc. For example, in a host/guest
architecture, the host might expose a device to the guest on a PCI bus,
and the guest will use a PCI-specific driver to interact with it.

The standard is split into sections describing general virtio
implementation and transport-specific sections."

> +
> +\section{Virtio Transport Requirements}\label{sec:Virtio Transport Options / 
> Virtio Transport Requirements}
> +
> +\devicenormative{\subsection}{Virtio Transport Requirements}{Virtio 
> Transport Options}

I'm not sure we can introduce MUST (NOT) requirements for basic
functionality after the spec has been published for quite a time already
(although I'd assume every implementation is fulfilling the requirements
anyway)... thoughts?

> +
> +The device MUST present each event, in a transport defined way, from the
> +moment it takes place until the driver acknowledges the event.

I don't believe "event" is well-defined here.

> +
> +The device MUST NOT access virtqueue's contents before the driver
> +notifies that the queue is ready for access, in a transport defined way.
> +
> +The device MUST NOT access buffers on the virtqueue, after it has
> +modified them and notified the driver about their availability.
> +
> +The device MUST reset the virtqueues if requested by the driver, in a
> +transport defined way.

Isn't all of this already defined in one place of the spec or another?

> +
> +\drivernormative{\subsection}{Virtio Transport Requirements}{Virtio 
> Transport Options}
> +
> +The driver MUST NOT access guest memory locations outside what's made
> +available by the device to the driver.

I don't think that makes sense -- I'd assume most guest memory locations
do not have anything to do with virtio, and we should try to avoid
host/guest terminology.

> +
> +The driver MUST NOT write to the read-only memory area and MUST NOT read
> +from the write-only memory area.

Which memory areas does that refer to? Parts of the transport-specific
data structures?

> +
> +The driver MUST acknowledge events presented by the device, as mandated
> +by the transport.

I don't think this is quite correct in the absolute -- for example, it
should be fine to not acknowledge events if some overriding event comes
along, or if the driver initiates a reset.

> +
> +The driver MUST NOT access virtqueue contents before the device notifies
> +about the readiness of the same.
> +
> +The driver MUST NOT access buffers, after it has added them to the
> +virtqueue and notified the device about their availability. The driver
> +MAY access them after the device has processed them and notified the
> +driver of their availability, in a transport defined way.
> +
> +The driver MAY ask the device to reset the virtqueues if, for example,
> +the driver times out waiting for a notification from the device for a
> +previously queued request.

Again, I believe this has already been covered in the generic
sections -- do we instead need to specify that a transport MUST provide
a method to do xy? (or 

Re: [virtio-dev] [PATCH V6 2/2] virtio-spi: add the device specification

[Cornelia Huck]

On Fri, Dec 01 2023, Haixu Cui  wrote:

>>> +Note: chipselect is an electrical signal controlled by the SPI controller, 
>>> used to select the
>>> +SPI slaves connected to the controller.
>> 
>> I wonder whether another term is now more commonly used... the Wikipedia
>> article uses main/sub, is there a term that is usually used together
>> with "controller"?
>
> In Wikipedia,
>
> "Historically, this arrangement has been called master/slave. But to 
> avoid referencing slavery, alternative names discussed in § Alternative 
> namings have been used."
>
> I am not quite sure if main/sub is widely used, but in latest Linux, 
> still use master/slave but master is gradually replaced by "controller".
>
> So I think "controller" here does make sense. What is your suggestion?

"controller" is fine, I'm just wondering if "slave" has started to be
replaced with something else as well... if it hasn't, we probably need
to stick with it to avoid confusing readers.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [virtio-dev] [PATCH V6 2/2] virtio-spi: add the device specification

[Cornelia Huck]

On Thu, Nov 30 2023, Haixu Cui  wrote:

> The Virtio SPI (Serial Peripheral Interface) device is a virtual
> SPI controller that allows the driver to operate and use the SPI
> controller under the control of the device.
>
> This patch adds the specification for virtio-spi.
>
> Signed-off-by: Haixu Cui 
> ---
>  device-types/spi/description.tex| 288 
>  device-types/spi/device-conformance.tex |   7 +
>  device-types/spi/driver-conformance.tex |   7 +
>  3 files changed, 302 insertions(+)
>  create mode 100644 device-types/spi/description.tex
>  create mode 100644 device-types/spi/device-conformance.tex
>  create mode 100644 device-types/spi/driver-conformance.tex
>
> diff --git a/device-types/spi/description.tex 
> b/device-types/spi/description.tex
> new file mode 100644
> index 000..c4816a6
> --- /dev/null
> +++ b/device-types/spi/description.tex
> @@ -0,0 +1,288 @@
> +\section{SPI Controller Device}\label{sec:Device Types / SPI Controller 
> Device}
> +
> +The Virtio SPI (Serial Peripheral Interface) device is a virtual SPI 
> controller that
> +allows the driver to operate and use the SPI controller under the control of 
> the device,
> +either a physical SPI controller, or an emulated one.
> +
> +The Virtio SPI device has a single virtqueue. SPI transfer requests are 
> placed into
> +the virtqueue, and serviced by the device.

I think it would still make sense to keep the host/guest example you
included in the last version; while we have to talk about device/driver
in the specification, giving host/guest as an example is completely
fine.

> +
> +\subsection{Device ID}\label{sec:Device Types / SPI Controller Device / 
> Device ID}
> +45
> +
> +\subsection{Virtqueues}\label{sec:Device Types / SPI Controller Device / 
> Virtqueues}
> +
> +\begin{description}
> +\item[0] requestq
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / SPI Controller Device / 
> Feature bits}
> +
> +None
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / SPI 
> Controller Device / Device configuration layout}
> +
> +All fields of this configuration are always available and read-only for 
> Virtio SPI driver.

s/for Virtio SPI driver/for the driver/

(matches what is said for other device types)

> +The config space shows the features and settings supported by the device.
> +
> +\begin{lstlisting}
> +struct virtio_spi_config {
> + u8 cs_max_number;
> + u8 cs_change_supported;
> + u8 tx_nbits_supported;
> + u8 rx_nbits_supported;
> + le32 bits_per_word_mask;
> + le32 mode_func_supported;
> + le32 max_freq_hz;
> + le32 max_word_delay_ns;
> + le32 max_cs_setup_ns;
> + le32 max_cs_hold_ns;
> + le32 max_cs_inactive_ns;
> +};
> +\end{lstlisting}
> +
> +\field{cs_max_number} is the maximum number of chipselect the device 
> supports.
> +
> +Note: chipselect is an electrical signal controlled by the SPI controller, 
> used to select the
> +SPI slaves connected to the controller.

I wonder whether another term is now more commonly used... the Wikipedia
article uses main/sub, is there a term that is usually used together
with "controller"?

> +
> +\field{cs_change_supported} indicates if the device supports to toggle 
> chipselect
> +after each transfer in one message:
> +0: unsupported, means chipselect will keep active when executing the 
> message transaction;
> +1: supported.
> +
> +Note: Message here contains a sequence of SPI transfers.
> +
> +\field{tx_nbits_supported} indicates the supported number of bit for 
> writing, SINGLE is always
> +supported. In the bit definition below, a set bit means the transfer is 
> supported, otherwise not:
> +bit 0: DUAL;
> +bit 1: QUAD;
> +bit 2: OCTAL;
> +other bits are reserved and must be set to 0 by the device.
> +
> +Note: Transfer bit options are commonly used in SPI:
> +\begin{itemize}
> +\item SINGLE: 1-bit transfer
> +\item DUAL: 2-bit transfer
> +\item QUAD: 4-bit transfer
> +\item OCTAL: 8-bit transfer
> +\end{itemize}
> +
> +\field{rx_nbits_supported} indicates the supported number of bit for 
> reading, SINGLE is always
> +supported. In the bit definition below, a set bit means the transfer is 
> supported, otherwise not:
> +bit 0: DUAL;
> +bit 1: QUAD;
> +bit 2: OCTAL;
> +other bits are reserved and must be set to 0 by the device.
> +
> +\field{bits_per_word_mask} is a mask indicating which values of 
> bits_per_word are supported.
> +If not set, no limitation for bits_per_word.

s/no limitation/there is no limitation/

> +
> +Note: bits_per_word corresponds to \field{bits_per_word} in \field{struct 
> virtio_spi_transfer_head}.
> +
> +\field{mode_func_supported} indicates whether the following features are 
> supported or not:
> +bit 0-1: CPHA feature,
> +0b00: supports CPHA=0 and CPHA=1;
> +0b01: supports CPHA=0 only;
> +0b10: 

Re: [PATCH v2 3/3] hw/misc/pvpanic: add support for normal shutdowns

[Cornelia Huck]

On Wed, Nov 29 2023, Thomas Weißschuh  wrote:
> On 2023-11-29 09:23:46+0100, Cornelia Huck wrote:
>> On Tue, Nov 28 2023, Thomas Weißschuh  wrote:
>> > diff --git a/include/standard-headers/linux/pvpanic.h 
>> > b/include/standard-headers/linux/pvpanic.h
>> > index 54b7485390d3..38e53ad45929 100644
>> > --- a/include/standard-headers/linux/pvpanic.h
>> > +++ b/include/standard-headers/linux/pvpanic.h
>> > @@ -5,5 +5,6 @@
>> >  
>> >  #define PVPANIC_PANICKED  (1 << 0)
>> >  #define PVPANIC_CRASH_LOADED  (1 << 1)
>> > +#define PVPANIC_SHUTDOWN  (1 << 2)
>> >  
>> >  #endif /* __PVPANIC_H__ */
>> >
>> 
>> This hunk needs to come in via a separate headers update, or has to be
>> split out into a placeholder patch if it is not included in the Linux
>> kernel yet.
>
> Greg KH actually want this header removed from the Linux UAPI headers,
> as it is not in fact a Linux UAPI [0].
> It's also a weird workflow to have the specification in qemu but the
> header as part of Linux that is re-imported in qemu.
>
> What do you think about maintaining the header as a private part of qemu
> and dropping it from Linux UAPI?
>
> Contrary to my response to Greg this wouldn't break old versions of
> qemu, as qemu is using a private copy that would still exist there.
>
> [0] https://lore.kernel.org/lkml/2023110431-pacemaker-pruning-0e4c@gregkh/

Hm... we have a bunch of examples where we use things exported via the
Linux uapi header files that are not a kernel<->userspace interface, but
rather a host<->guest interface (sometimes defining the interface,
sometimes more as a convenience mechanism). I agree that this is not
quite what the Linux uapi is supposed to be (and yes, it's weird), but
we've being doing that for many years now and changing it would be a
non-zero effort (and we'd have to figure out another way to make sure
the kernel and QEMU do not diverge if there's no authorative third party
around.)

In the case of the pvpanic device, this seems manageable, though; if we
decide to go that way, we should

1. copy the header on the QEMU side somewhere else under include/ and
   remove it from the header update script
2. wait until this hits QEMU mainline (so nobody will try to run the old
   update script)
3. move the uapi file on the Linux side

(We've had changes in the kernel break the update script before, but if
we can do it more smoothly, I'd prefer that way -- the kernel merge
window won't open before the new year anyway, and by that time, we'll
have the QEMU tree open again.)

Main downside is that you'd have extra hassle for something that looks
like a straightforward feature, which is not ideal. (Also, are we sure
that nobody else consumes that header file?)

I'm not sure if dealing with the other host<->guest interfaces that get
copied over is worth the effort, though...

Opinions?

[virtio-dev] Re: [virtio-comment] [PATCH] [PATCH v5] virtio-spi: add the device specification

[Cornelia Huck]

On Wed, Nov 29 2023, Viresh Kumar  wrote:

> On 29-11-23, 18:31, Haixu Cui wrote:
>> Hi Viresh,
>> 
>> Thank you for your helpful comments. In next patch, I will clearly point
>> this out:
>
> Great, finally we are on the same page. Thanks Haixu.
>
>> "For full-duplex read and write transfer, both \field{tx_buf} and
>> \field{rx_buf} are used and the buffer size of \field{tx_buf} must be same
>> as \field{rx_buf}."
>
> Suggest rewriting as:
>
> In full-duplex transfer mode, both \field{tx_buf} and \field{rx_buf} are sent 
> by
> the driver, \field{tx_buf} followed by \field{rx_buf}. The length of both the
> buffers MUST be same.

Is that in a non-normative section? (Sorry, I've lost track here...) If
so, I would say:

"The length of both buffers has to be the same."

>
>> And in drivernormative section, I will add a requirement:
>> "For full-duplex transfer, Virtio SPI driver MUST guarantee the write
>> transfer size is equal to the read transfer size"
>
> Maybe:
>
> drivernormative:
>
> In full-duplex transfer mode, the Virtio SPI driver MUST guarantee that the
> length of both \field{tx_buf} and \field{rx_buf} are same.

s/are same/is the same/

>
> devicenormative:
>
> In full-duplex transfer mode, the Virtio SPI device MUST verify that the 
> length
> of both \field{tx_buf} and \field{rx_buf} are same. In case of any mismatch, 
> the

s/are same/is the same/

> device MUST fail the transfer and notify the driver.

"notify the driver" == "set an appropriate error value"?

Can this be covered by one of the existing error values?


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH] [PATCH v5] virtio-spi: add the device specification

[Cornelia Huck]

On Wed, Nov 29 2023, Haixu Cui  wrote:

> Hi Viresh,
>
> On 11/29/2023 3:30 PM, Viresh Kumar wrote:
>> On 28-11-23, 20:58, Haixu Cui wrote:
>>> On 11/27/2023 6:17 PM, Viresh Kumar wrote:
 On 24-11-23, 15:20, Haixu Cui wrote:
> +For half-duplex read transfer, \field{rx_buf} is filled by Virtio SPI 
> device and consumed
> +by Virtio SPI driver. For half-duplex write transfer, \field{tx_buf} is 
> filled by Virtio
> +SPI driver and consumed by Virtio SPI device. And for full-duplex read 
> and write transfer,
> +both \field{tx_buf} and \field{rx_buf} are used.

 Should the length of both the buffers in full-duplex mode be same ? If 
 yes, then
 this should be mentioned (in case it is not).

>>>
>>> No, there is no such limitation. Write and read buffers may be different is
>>> size.
>> 
>> Hmm, I worked with a SPI controller over a decade ago, and I must be 
>> forgetting
>> something here I guess. But from whatever little I remember, with full-duplex
>> transfer, data flows on both MOSI and MISO lines as soon as clock signal is
>> applied.  And so amount of data sent is always be equal to amount of data
>> received by both sides.
>> 
>> Also if I see Linux's implementation of the `struct spi_transfer` [1], I see
>> `tx_buf`, `rx_buf` and a single `len` field, which applies to both the 
>> buffers.
>> Which I guess is indicating that both buffers are supposed to be of same 
>> length.
>> 
>> What am I missing ?
>
> Oh so sorry for that. And I don't make it clear. Yes, tx_buf and rx_buf 
> have the same size, Linux has such restriction. Just as you mention, 
> kernel level spi_transfer has single "len", the same for 
> spi_ioc_transfer passed from the userland.
>
> But I am not sure if this is in the scope of the spec. Because this is 
> ensured by Linux, but Virtio SPI driver won't also can't verify this.
> This is a prerequisite for virtio spi processing requests.
>
> What is your suggestion? How about adding some descriptions here, like 
> "for full-duplex, tx_buf and rx_buf are same in size, this is guaranteed 
> by the kernel"?

We must not really make any assumptions in the spec about concrete
implementations (here, the Linux kernel), as someone implementing it in
a different environment will need to make explicit choices.

So, if tx_buf and rx_buf are required to be of the same size, it needs
to be explicitly stated in the spec, or an implementation might choose
to do it differently.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [PATCH v2 3/3] hw/misc/pvpanic: add support for normal shutdowns

[Cornelia Huck]

On Tue, Nov 28 2023, Thomas Weißschuh  wrote:

> Shutdown requests are normally hardware dependent.
> By extending pvpanic to also handle shutdown requests, guests can
> submit such requests with an easily implementable and cross-platform
> mechanism.
>
> Signed-off-by: Thomas Weißschuh 
> ---
>  docs/specs/pvpanic.rst   | 2 ++
>  hw/misc/pvpanic.c| 5 +
>  include/hw/misc/pvpanic.h| 2 +-
>  include/standard-headers/linux/pvpanic.h | 1 +
>  4 files changed, 9 insertions(+), 1 deletion(-)

(...)

> diff --git a/include/standard-headers/linux/pvpanic.h 
> b/include/standard-headers/linux/pvpanic.h
> index 54b7485390d3..38e53ad45929 100644
> --- a/include/standard-headers/linux/pvpanic.h
> +++ b/include/standard-headers/linux/pvpanic.h
> @@ -5,5 +5,6 @@
>  
>  #define PVPANIC_PANICKED (1 << 0)
>  #define PVPANIC_CRASH_LOADED (1 << 1)
> +#define PVPANIC_SHUTDOWN (1 << 2)
>  
>  #endif /* __PVPANIC_H__ */
>

This hunk needs to come in via a separate headers update, or has to be
split out into a placeholder patch if it is not included in the Linux
kernel yet.

Re: [PATCH for-8.2] target/arm: Disable SME if SVE is disabled

[Cornelia Huck]

On Mon, Nov 27 2023, Peter Maydell  wrote:

> There is no architectural requirement that SME implies SVE, but
> our implementation currently assumes it. (FEAT_SME_FA64 does
> imply SVE.) So if you try to run a CPU with eg "-cpu max,sve=off"
> you quickly run into an assert when the guest tries to write to
> SMCR_EL1:
>
> #6  0x74b38e96 in __GI___assert_fail
> (assertion=0x566e69cb "sm", file=0x566e5b24 
> "../../target/arm/helper.c", line=6865, function=0x566e82f0 
> <__PRETTY_FUNCTION__.31> "sve_vqm1_for_el_sm") at ./assert/assert.c:101
> #7  0x55ee33aa in sve_vqm1_for_el_sm (env=0x57d291f0, el=2, 
> sm=false) at ../../target/arm/helper.c:6865
> #8  0x55ee3407 in sve_vqm1_for_el (env=0x57d291f0, el=2) at 
> ../../target/arm/helper.c:6871
> #9  0x55ee3724 in smcr_write (env=0x57d291f0, ri=0x57da23b0, 
> value=2147483663) at ../../target/arm/helper.c:6995
> #10 0x55fd1dba in helper_set_cp_reg64 (env=0x57d291f0, 
> rip=0x57da23b0, value=2147483663) at ../../target/arm/tcg/op_helper.c:839
> #11 0x7fff60056781 in code_gen_buffer ()
>
> Avoid this unsupported and slightly odd combination by
> disabling SME when SVE is not present.
>
> Cc: qemu-sta...@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2005
> Signed-off-by: Peter Maydell 
> ---
> '-cpu sve=off,sme=on,sme_fa64=off' crashes in the same way, so just
> turning off FA64 isn't sufficient.  Maybe we should support
> SME-no-SVE, but for 8.2 at least turning off SME is better than
> letting users hit an assertion.
> ---
>  target/arm/cpu.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 25e9d2ae7b8..0fe268ac785 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1743,6 +1743,15 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error 
> **errp)
>  return;
>  }
>  
> +/*
> + * FEAT_SME is not architecturally dependent on FEAT_SVE (unless
> + * FEAT_SME_FA64 is present). However our implementation currently
> + * assumes it, so if the user asked for sve=off then turn off SME 
> also.
> + */

Might be worth adding a note here that KVM currently does not support
SME anyway? It took me a moment to remember that.

> +if (cpu_isar_feature(aa64_sme, cpu) && !cpu_isar_feature(aa64_sve, 
> cpu)) {
> +object_property_set_bool(OBJECT(cpu), "sme", false, 
> _abort);
> +}
> +
>  arm_cpu_sme_finalize(cpu, _err);
>  if (local_err != NULL) {
>  error_propagate(errp, local_err);

[virtio-dev] Re: [virtio-comment] Re: [PATCH] [PATCH v5] virtio-spi: add the device specification

[Cornelia Huck]

On Mon, Nov 27 2023, Cornelia Huck  wrote:

> On Mon, Nov 27 2023, Haixu Cui  wrote:
>
>> On 11/24/2023 11:46 PM, Cornelia Huck wrote:
>>> On Fri, Nov 24 2023, Haixu Cui  wrote:
>>>> +The \field{chip_select_max_number} is the maximum number of chipselect 
>>>> the host SPI controller supports.
>>> 
>>> "chipselect" is probably a known term for people familiar with SPI -- is
>>> there any definition of those terms that the spec can point to?
>>
>> Just as Mark said, there is no formal spec for SPI, so no standard spec 
>> for such terms referring to. The same for CPHA/CPOL/LSB/MSB, please see 
>> below.
>
> If we have nothing to point to, it is probably best to simply
> expand/explain the terms on their first usage.
>
>>> Can we point to some documentation that explains CPHA and CPOL?
>>
>> Here. No standard SPI spec to point to. CPOL/CPHA have definitions in 
>> wikipedia(Clock polarity and phase chapter):
>>
>> https://en.wikipedia.org/wiki/Serial_Peripheral_Interface
>>
>> How about copying some concise information from wikipedia as Note? Or is 
>> referring to such webpage acceptable in this spec.
>
> Not sure if we can do an outright copy (licence compatibility), but
> paraphrasing should be fine. (I'd rather not directly reference the
> site, because the content is not guaranteed to be stable, but we could
> maybe add it as "further reading".)

Looking again, the kernel doc referenced by Mark
(https://www.kernel.org/doc/html/v6.6/driver-api/spi.html) might also be
a good candidate, especially as we can refer to a specific version.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH] [PATCH v5] virtio-spi: add the device specification

[Cornelia Huck]

On Mon, Nov 27 2023, Haixu Cui  wrote:

> On 11/24/2023 11:46 PM, Cornelia Huck wrote:
>> On Fri, Nov 24 2023, Haixu Cui  wrote:
>>> +The \field{chip_select_max_number} is the maximum number of chipselect the 
>>> host SPI controller supports.
>> 
>> "chipselect" is probably a known term for people familiar with SPI -- is
>> there any definition of those terms that the spec can point to?
>
> Just as Mark said, there is no formal spec for SPI, so no standard spec 
> for such terms referring to. The same for CPHA/CPOL/LSB/MSB, please see 
> below.

If we have nothing to point to, it is probably best to simply
expand/explain the terms on their first usage.

>> Can we point to some documentation that explains CPHA and CPOL?
>
> Here. No standard SPI spec to point to. CPOL/CPHA have definitions in 
> wikipedia(Clock polarity and phase chapter):
>
> https://en.wikipedia.org/wiki/Serial_Peripheral_Interface
>
> How about copying some concise information from wikipedia as Note? Or is 
> referring to such webpage acceptable in this spec.

Not sure if we can do an outright copy (licence compatibility), but
paraphrasing should be fine. (I'd rather not directly reference the
site, because the content is not guaranteed to be stable, but we could
maybe add it as "further reading".)

>>> +For each transfer request, Virtio SPI driver MUST check the fields in 
>>> structure \field{virtio_spi_transfer_head}
>>> +and MUST reject the request if any filed is invalid or enabling the 
>>> features not supported by host.
>> 
>> s/filed/field/
>> s/host/device/
>> 
>> Also, isn't the rejecting supposed to be done by the device, as the
>> driver is the party enqueueing the requests? Or do I have some kind of
>> fundamental misunderstanding?
>
> It may be better to filter some invalid requests by driver, as in the 
> request header there are many parameters, and some of them are not 
> supported by device, so it's quite possible that many requests invalid 
> for the device. So if driver can do the first filter, such invalid 
> requests will not be sent at all, this will conserve virtqueue and 
> system overhead.
>
> And this is why exposing device supported features in the config space, 
> it ensures that almost all requests in virtqueue are nice to the backend.
>
> device also will verify the requests again, as the following requirement:
> Virtio SPI device MUST verify the parameters in 
> \field{virtio_spi_transfer_head} after receiving the request,
> and MUST set \field{virtio_spi_transfer_result} as VIRTIO_SPI_PARAM_ERR 
> if not all parameters are valid or some device unsupported features are set.
>
> Although checking the requests twice seems a little redundant, it is 
> more efficient comparing with sending some invalid requests to the device.
>
> What is your opinion? Do you think it is acceptable?

Thanks for your explanation, I think we simply have some terminology
issues. In the virtio spec, "driver" refers to one side of the
driver/device pair, and is used to describe how to communicate with the
device. In this case, "driver" would be the entity interacting with the
device, regardless of how it is implemented, and would be responsible
for sending the requests. Any filtering that would be done in a concrete
implementation (i.e. if you have one component generating the requests,
and then another component filtering them before actually putting them
into the queue) is out of scope for this spec -- we can maybe specify
that the driver should not send invalid requests, but I'm not sure that
this is actually needed.

> Once again, thanks a lot for your support.

You're welcome!


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH] [PATCH v5] virtio-spi: add the device specification

[Cornelia Huck]

On Fri, Nov 24 2023, Haixu Cui  wrote:

> virtio-spi is a virtual SPI (Serial Peripheral Interface) controller and it 
> allows
> a guest to operate and use the host SPI controller.
>
> This patch adds the specification for virtio-spi.

As Mark has already reviewed it from the SPI POV (thanks!), I'm now
looking at the virtio side.

>
> Signed-off-by: Haixu Cui 
> ---
>  device-types/spi/description.tex| 281 
>  device-types/spi/device-conformance.tex |   7 +
>  device-types/spi/driver-conformance.tex |   7 +
>  3 files changed, 295 insertions(+)
>  create mode 100644 device-types/spi/description.tex
>  create mode 100644 device-types/spi/device-conformance.tex
>  create mode 100644 device-types/spi/driver-conformance.tex
>
> diff --git a/device-types/spi/description.tex 
> b/device-types/spi/description.tex
> new file mode 100644
> index 000..8ca8c0d
> --- /dev/null
> +++ b/device-types/spi/description.tex
> @@ -0,0 +1,281 @@
> +\section{SPI Master Device}\label{sec:Device Types / SPI Master Device}
> +
> +virtio-spi is a virtual SPI (Serial Peripheral Interface) controller and it 
> allows
> +a guest to operate and use the host SPI controller. Host SPI controller may 
> be a
> +physical controller, or a virtual one(e.g. controller emulated by the 
> software

Missing space before the opening bracket.

> +running in the host).
> +
> +virtio-spi has a single virtqueue. SPI transfer requests are placed into
> +the virtqueue, and serviced by the host SPI controller.

Is there any way to express all of this without referring to "host" and
"guest" here? (The paragraph below giving it as an example is fine.)

Something like "The virtio-spi device is a virtual SPI controller. It
allows the driver to operate and use an SPI controller under the control
of the device, either a physical controller, or an emulated one."

> +
> +In a typical host and guest architecture with virtio-spi, Virtio SPI driver 
> is
> +the front-end running in the guest kernel, and Virtio SPI device acts as the
> +back-end in the host.
> +
> +\subsection{Device ID}\label{sec:Device Types / SPI Master Device / Device 
> ID}
> +45
> +
> +\subsection{Virtqueues}\label{sec:Device Types / SPI Master Device / 
> Virtqueues}
> +
> +\begin{description}
> +\item[0] requestq
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / SPI Master Device / 
> Feature bits}
> +
> +None
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / SPI Master 
> Device / Device configuration layout}
> +
> +All fields of this configuration are always available and read-only for 
> Virtio SPI driver.
> +The config space shows the features and settings supported by the host SPI 
> controller.
> +
> +\begin{lstlisting}
> +struct virtio_spi_config {
> + u8 chip_select_max_number;
> + u8 cs_change_supported;
> + u8 tx_nbits_supported;
> + u8 rx_nbits_supported;
> + le32 bits_per_word_mask;
> + le32 mode_func_supported;
> + le32 max_freq_hz;
> + le32 max_word_delay_ns;
> + le32 max_cs_setup_ns;
> + le32 max_cs_hold_ns;
> + le32 max_cs_inactive_ns;
> +};
> +\end{lstlisting}
> +
> +The \field{chip_select_max_number} is the maximum number of chipselect the 
> host SPI controller supports.

"chipselect" is probably a known term for people familiar with SPI -- is
there any definition of those terms that the spec can point to?

Also, it should be either "The field \field{whatever}" or
"\field{whatever}"; "The \field{whatever}" looks good in the LaTeX
source, but not in the end result.

> +
> +The \field{cs_change_supported} indicates if the host SPI controller 
> supports to toggle chipselect
> +after each transfer in one message:
> +0: supported;
> +1: unsupported, means chipselect will keep active when executing the 
> message transaction.
> +Note: Message here contains a sequence of SPI transfer.
> +
> +The \field{tx_nbits_supported} indicates the supported number of bit for 
> writing:
> +bit 0: 2-bit transfer, 0 for unsupported and 1 for supported;
> +bit 1: 4-bit transfer, 0 for unsupported and 1 for supported;
> +bit 2: 8-bit transfer, 0 for unsupported and 1 for supported;
> +other bits are reserved as 0, 1-bit transfer is always supported.
> +
> +The \field{rx_nbits_supported} indicates the supported number of bit for 
> reading:
> +bit 0: 2-bit transfer, 0 for unsupported and 1 for supported;
> +bit 1: 4-bit transfer, 0 for unsupported and 1 for supported;
> +bit 2: 8-bit transfer, 0 for unsupported and 1 for supported;
> +other bits are reserved as 0, 1-bit transfer is always supported.
> +
> +The \field{bits_per_word_mask} is a mask indicating which values of 
> bits_per_word
> +are supported. If not set, no limitation for bits_per_word.
> +Note: bits_per_word corresponds to \field{bits_per_word} field in
> +structure \field{virtio_spi_transfer_head}, see below.
> +
> 

[PATCH for-9.0] hw: Add compat machines for 9.0

[Cornelia Huck]

Add 9.0 machine types for arm/i440fx/m68k/q35/s390x/spapr.

Signed-off-by: Cornelia Huck 
---
 hw/arm/virt.c  |  9 -
 hw/core/machine.c  |  3 +++
 hw/i386/pc.c   |  3 +++
 hw/i386/pc_piix.c  | 17 ++---
 hw/i386/pc_q35.c   | 13 -
 hw/m68k/virt.c |  9 -
 hw/ppc/spapr.c | 15 +--
 hw/s390x/s390-virtio-ccw.c | 14 +-
 include/hw/boards.h|  3 +++
 include/hw/i386/pc.h   |  3 +++
 10 files changed, 80 insertions(+), 9 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index be2856c018aa..efd503d45e48 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3180,10 +3180,17 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
+static void virt_machine_9_0_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(9, 0)
+
 static void virt_machine_8_2_options(MachineClass *mc)
 {
+virt_machine_9_0_options(mc);
+compat_props_add(mc->compat_props, hw_compat_8_2, hw_compat_8_2_len);
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(8, 2)
+DEFINE_VIRT_MACHINE(8, 2)
 
 static void virt_machine_8_1_options(MachineClass *mc)
 {
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0c1739814124..2699bcba5357 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -32,6 +32,9 @@
 #include "hw/virtio/virtio-net.h"
 #include "audio/audio.h"
 
+GlobalProperty hw_compat_8_2[] = {};
+const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
+
 GlobalProperty hw_compat_8_1[] = {
 { TYPE_PCI_BRIDGE, "x-pci-express-writeable-slt-bug", "true" },
 { "ramfb", "x-migrate", "off" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 29b9964733ed..496498df3a8f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -78,6 +78,9 @@
 { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
 { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
 
+GlobalProperty pc_compat_8_2[] = {};
+const size_t pc_compat_8_2_len = G_N_ELEMENTS(pc_compat_8_2);
+
 GlobalProperty pc_compat_8_1[] = {};
 const size_t pc_compat_8_1_len = G_N_ELEMENTS(pc_compat_8_1);
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index eace8543358a..042c13cdbc33 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -545,13 +545,26 @@ static void pc_i440fx_machine_options(MachineClass *m)
  "Use a different south bridge than 
PIIX3");
 }
 
-static void pc_i440fx_8_2_machine_options(MachineClass *m)
+static void pc_i440fx_9_0_machine_options(MachineClass *m)
 {
 pc_i440fx_machine_options(m);
 m->alias = "pc";
 m->is_default = true;
 }
 
+DEFINE_I440FX_MACHINE(v9_0, "pc-i440fx-9.0", NULL,
+  pc_i440fx_9_0_machine_options);
+
+static void pc_i440fx_8_2_machine_options(MachineClass *m)
+{
+pc_i440fx_9_0_machine_options(m);
+m->alias = NULL;
+m->is_default = false;
+
+compat_props_add(m->compat_props, hw_compat_8_2, hw_compat_8_2_len);
+compat_props_add(m->compat_props, pc_compat_8_2, pc_compat_8_2_len);
+}
+
 DEFINE_I440FX_MACHINE(v8_2, "pc-i440fx-8.2", NULL,
   pc_i440fx_8_2_machine_options);
 
@@ -560,8 +573,6 @@ static void pc_i440fx_8_1_machine_options(MachineClass *m)
 PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
 
 pc_i440fx_8_2_machine_options(m);
-m->alias = NULL;
-m->is_default = false;
 pcmc->broken_32bit_mem_addr_check = true;
 
 compat_props_add(m->compat_props, hw_compat_8_1, hw_compat_8_1_len);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 4f3e5412f6b8..f43d5142b8e5 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -383,12 +383,23 @@ static void pc_q35_machine_options(MachineClass *m)
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
 }
 
-static void pc_q35_8_2_machine_options(MachineClass *m)
+static void pc_q35_9_0_machine_options(MachineClass *m)
 {
 pc_q35_machine_options(m);
 m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v9_0, "pc-q35-9.0", NULL,
+   pc_q35_9_0_machine_options);
+
+static void pc_q35_8_2_machine_options(MachineClass *m)
+{
+pc_q35_9_0_machine_options(m);
+m->alias = NULL;
+compat_props_add(m->compat_props, hw_compat_8_2, hw_compat_8_2_len);
+compat_props_add(m->compat_props, pc_compat_8_2, pc_compat_8_2_len);
+}
+
 DEFINE_Q35_MACHINE(v8_2, "pc-q35-8.2", NULL,
pc_q35_8_2_machine_options);
 
diff --git a/hw/m68k/virt.c b/hw/m68k/virt.c
index 2e49e262ee0e..e2792ef46d93 100644
--- a/hw/m68k/virt.c
+++ b/hw/m68k/virt.c
@@ -346,10 +346,17 @@ type_init(virt_machine_register_types)
 } \
 type_init(machvirt_machine_##major##_##minor##_init);
 
+static void virt_

Re: [virtio] Invitation to comment on Virtual I/O Device (VIRTIO) Version 1.3 - ends December 8th

[Cornelia Huck]

It seems that the original mail did not make it to some of the mailing
lists it was intended to go to, possibly due to the html part of the
original mail. Therefore, I'm trying again, this time with plain text
only. Apologies in advance for any duplicates.

On Wed, Nov 08 2023, Paul Knight  wrote:

> OASIS members and other interested parties,
>
> OASIS and the OASIS Virtual I/O Device (VIRTIO) TC are pleased to announce
> that Virtual I/O Device (VIRTIO) Version 1.3 is now available for public
> review and comment.
>
> Specification Overview:
>
> This document describes the specifications of the 'virtio' family of
> devices. These devices are found in virtual environments, yet by design
> they look like physical devices to the guest within the virtual machine -
> and this document treats them as such. This similarity allows the guest to
> use standard drivers and discovery mechanisms. The purpose of virtio and
> this specification is that virtual environments and guests should have a
> straightforward, efficient, standard and extensible mechanism for virtual
> devices, rather than boutique per-environment or per-OS mechanisms.
>
> The documents and related files are available here:
>
> Virtual I/O Device (VIRTIO) Version 1.3
> Committee Specification Draft 01
> 06 October 2023
>
> Editable source (Authoritative):
> https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/tex/
> HTML:
> https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.html
> PDF:
> https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.pdf
> Example driver listing:
> https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/listings/
> PDF file marked to indicate changes from Version 1.2 Committee
> Specification 01:
> https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01-diff-from-v1.2-cs01.pdf
>
> For your convenience, OASIS provides a complete package of the
> specification document and any related files in ZIP distribution files. You
> can download the ZIP file at:
> https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01.zip
>
> A public review metadata record documenting this and any previous public
> reviews is available at:
> https://docs.oasis-open.org/virtio/virtio/v1.3/csd01/virtio-v1.3-csd01-public-review-metadata.html
>
> How to Provide Feedback
>
> OASIS and the OASIS Virtual I/O Device (VIRTIO) TC value your feedback. We
> solicit input from developers, users and others, whether OASIS members or
> not, for the sake of improving the interoperability and quality of its
> technical work.
>
> The public review starts 09 November 2023 at 00:00 UTC and ends 08 December
> 2023 at 23:59 UTC.
>
> Comments may be submitted to the TC by any person through the use of the
> OASIS TC Comment Facility which can be used by following the instructions
> on the TC's "Send A Comment" page (
> https://www.oasis-open.org/committees/comments/index.php?wg_abbrev=virtio).
>
> Comments submitted by TC non-members for this work and for other work of
> this TC are publicly archived and can be viewed at:
>
> https://lists.oasis-open.org/archives/virtio-comment/
>
> All comments submitted to OASIS are subject to the OASIS Feedback License,
> which ensures that the feedback you provide carries the same obligations at
> least as the obligations of the TC members. In connection with this public
> review, we call your attention to the OASIS IPR Policy [1] applicable
> especially [2] to the work of this technical committee. All members of the
> TC should be familiar with this document, which may create obligations
> regarding the disclosure and availability of a member's patent, copyright,
> trademark and license rights that read on an approved OASIS specification.
>
> OASIS invites any persons who know of any such claims to disclose these if
> they may be essential to the implementation of the above specification, so
> that notice of them may be posted to the notice page for this TC's work.
>
> Additional information about the specification and the VIRTIO TC can be
> found at the TC's public home page:
> https://www.oasis-open.org/committees/virtio/
>
> == Additional references:
>
> [1] https://www.oasis-open.org/policies-guidelines/ipr/
>
> [2] https://github.com/oasis-tcs/virtio-admin/blob/master/IPR.md
> https://www.oasis-open.org/policies-guidelines/ipr/#Non-Assertion-Mode
> Non-Assertion Mode
> -- 
> Paul Knight Document Process Analyst
> 
> OASIS ...Setting the standard for open
> collaboration

Re: [PATCH v1 1/9] linux-headers: Add KVM headers for loongarch

[Cornelia Huck]

On Wed, Nov 08 2023, xianglai li  wrote:

> From: zhaotianrui 
>
> Update linux KVM headers about LoongArch KVM form linux
> kernel. Mainly contains some KVM structures and macro
> defines such as LoongArch KVM registers number, LoongArch
> fpu structure, exit reason of LoongArch IOCSR, etc.
>
> Signed-off-by: zhaotianrui 
> Signed-off-by: xianglai li 
> ---
>  linux-headers/asm-loongarch/kvm.h | 108 ++
>  linux-headers/linux/kvm.h |   9 +++
>  2 files changed, 117 insertions(+)
>  create mode 100644 linux-headers/asm-loongarch/kvm.h

This needs to be a full headers sync with a defined Linux kernel version
(e.g. with -rc1), not just the LoongArch parts.

Re: [PATCH v2 3/3] arm/kvm: convert to read_sys_reg64

[Cornelia Huck]

On Tue, Oct 17 2023, Peter Maydell  wrote:

> On Tue, 10 Oct 2023 at 15:25, Cornelia Huck  wrote:
>>
>> We can use read_sys_reg64 to get the SVE_VLS register instead of
>> calling GET_ONE_REG directly.
>>
>> Suggested-by: Gavin Shan 
>> Signed-off-by: Cornelia Huck 
>> ---
>>  target/arm/kvm64.c | 6 +-
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>> index 558c0b88dd69..d40c89a84752 100644
>> --- a/target/arm/kvm64.c
>> +++ b/target/arm/kvm64.c
>> @@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>>  .target = -1,
>>  .features[0] = (1 << KVM_ARM_VCPU_SVE),
>>  };
>> -struct kvm_one_reg reg = {
>> -.id = KVM_REG_ARM64_SVE_VLS,
>> -.addr = (uint64_t)[0],
>> -};
>>  int fdarray[3], ret;
>>
>>  probed = true;
>> @@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
>>  error_report("failed to create scratch VCPU with SVE enabled");
>>  abort();
>>  }
>> -ret = ioctl(fdarray[2], KVM_GET_ONE_REG, );
>> +ret = read_sys_reg64(fdarray[2], [0], KVM_REG_ARM64_SVE_VLS);
>>  kvm_arm_destroy_scratch_host_vcpu(fdarray);
>>  if (ret) {
>>  error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
>
> read_sys_reg64() asserts that the register you're trying to
> read is 64 bits, but KVM_REG_ARM64_SVE_VLS is not, it's 512 bits:
>
> #define KVM_REG_ARM64_SVE_VLS   (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \
>  KVM_REG_SIZE_U512 | 0x)
>
> So this change would trip the assert on a host where SVE
> is supported and enabled.

Whoops, it seems that I misread this. (And my test environment didn't
have that enabled...)

[PATCH v2 2/3] arm/kvm: convert to kvm_get_one_reg

[Cornelia Huck]

We can neaten the code by switching the callers that work on a
CPUstate to the kvm_get_one_reg function.

Reviewed-by: Gavin Shan 
Signed-off-by: Cornelia Huck 
---
 target/arm/kvm.c   | 15 +++-
 target/arm/kvm64.c | 57 --
 2 files changed, 18 insertions(+), 54 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 1a8084c4601c..7903e2ddde1b 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -553,24 +553,19 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
 bool ok = true;
 
 for (i = 0; i < cpu->cpreg_array_len; i++) {
-struct kvm_one_reg r;
 uint64_t regidx = cpu->cpreg_indexes[i];
 uint32_t v32;
 int ret;
 
-r.id = regidx;
-
 switch (regidx & KVM_REG_SIZE_MASK) {
 case KVM_REG_SIZE_U32:
-r.addr = (uintptr_t)
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, regidx, );
 if (!ret) {
 cpu->cpreg_values[i] = v32;
 }
 break;
 case KVM_REG_SIZE_U64:
-r.addr = (uintptr_t)(cpu->cpreg_values + i);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, regidx, cpu->cpreg_values + i);
 break;
 default:
 g_assert_not_reached();
@@ -706,17 +701,13 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
 void kvm_arm_get_virtual_time(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
-struct kvm_one_reg reg = {
-.id = KVM_REG_ARM_TIMER_CNT,
-.addr = (uintptr_t)>kvm_vtime,
-};
 int ret;
 
 if (cpu->kvm_vtime_dirty) {
 return;
 }
 
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, KVM_REG_ARM_TIMER_CNT, >kvm_vtime);
 if (ret) {
 error_report("Failed to get KVM_REG_ARM_TIMER_CNT");
 abort();
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 047b269a7918..558c0b88dd69 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -909,14 +909,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 static int kvm_arch_get_fpsimd(CPUState *cs)
 {
 CPUARMState *env = _CPU(cs)->env;
-struct kvm_one_reg reg;
 int i, ret;
 
 for (i = 0; i < 32; i++) {
 uint64_t *q = aa64_vfp_qreg(env, i);
-reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-reg.addr = (uintptr_t)q;
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), q);
 if (ret) {
 return ret;
 } else {
@@ -940,15 +937,12 @@ static int kvm_arch_get_sve(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
 CPUARMState *env = >env;
-struct kvm_one_reg reg;
 uint64_t *r;
 int n, ret;
 
 for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) {
 r = >vfp.zregs[n].d[0];
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r);
 if (ret) {
 return ret;
 }
@@ -957,9 +951,7 @@ static int kvm_arch_get_sve(CPUState *cs)
 
 for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) {
 r = >vfp.pregs[n].p[0];
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_PREG(n, 0);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_PREG(n, 0), r);
 if (ret) {
 return ret;
 }
@@ -967,9 +959,7 @@ static int kvm_arch_get_sve(CPUState *cs)
 }
 
 r = >vfp.pregs[FFR_PRED_NUM].p[0];
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_FFR(0);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_FFR(0), r);
 if (ret) {
 return ret;
 }
@@ -980,7 +970,6 @@ static int kvm_arch_get_sve(CPUState *cs)
 
 int kvm_arch_get_registers(CPUState *cs)
 {
-struct kvm_one_reg reg;
 uint64_t val;
 unsigned int el;
 uint32_t fpr;
@@ -990,31 +979,24 @@ int kvm_arch_get_registers(CPUState *cs)
 CPUARMState *env = >env;
 
 for (i = 0; i < 31; i++) {
-reg.id = AARCH64_CORE_REG(regs.regs[i]);
-reg.addr = (uintptr_t) >xregs[i];
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]),
+  >xregs[i]);
 if (ret) {
 return ret;
 }
 }
 
-reg.id = AARCH64_CORE_REG(regs.sp);
-reg.addr = (uintptr_t) >sp_el[0];
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.sp), >sp_el[0]);
 if (ret) {
 return ret;
 }
 
-reg.id = AARCH64_CORE_REG(sp_el1);
-reg.addr = (uintptr_t) >sp_

[PATCH v2 1/3] arm/kvm: convert to kvm_set_one_reg

[Cornelia Huck]

We can neaten the code by switching to the kvm_set_one_reg function.

Reviewed-by: Gavin Shan 
Signed-off-by: Cornelia Huck 
---
 target/arm/kvm.c   | 13 +++--
 target/arm/kvm64.c | 66 +-
 2 files changed, 21 insertions(+), 58 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index b66b936a9586..1a8084c4601c 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -589,7 +589,6 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
 bool ok = true;
 
 for (i = 0; i < cpu->cpreg_array_len; i++) {
-struct kvm_one_reg r;
 uint64_t regidx = cpu->cpreg_indexes[i];
 uint32_t v32;
 int ret;
@@ -598,19 +597,17 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
 continue;
 }
 
-r.id = regidx;
 switch (regidx & KVM_REG_SIZE_MASK) {
 case KVM_REG_SIZE_U32:
 v32 = cpu->cpreg_values[i];
-r.addr = (uintptr_t)
+ret = kvm_set_one_reg(cs, regidx, );
 break;
 case KVM_REG_SIZE_U64:
-r.addr = (uintptr_t)(cpu->cpreg_values + i);
+ret = kvm_set_one_reg(cs, regidx, cpu->cpreg_values + i);
 break;
 default:
 g_assert_not_reached();
 }
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
 if (ret) {
 /* We might fail for "unknown register" and also for
  * "you tried to set a register which is constant with
@@ -731,17 +728,13 @@ void kvm_arm_get_virtual_time(CPUState *cs)
 void kvm_arm_put_virtual_time(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
-struct kvm_one_reg reg = {
-.id = KVM_REG_ARM_TIMER_CNT,
-.addr = (uintptr_t)>kvm_vtime,
-};
 int ret;
 
 if (!cpu->kvm_vtime_dirty) {
 return;
 }
 
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ret = kvm_set_one_reg(cs, KVM_REG_ARM_TIMER_CNT, >kvm_vtime);
 if (ret) {
 error_report("Failed to set KVM_REG_ARM_TIMER_CNT");
 abort();
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 5e95c496bb9f..047b269a7918 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -540,14 +540,10 @@ static int kvm_arm_sve_set_vls(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
 uint64_t vls[KVM_ARM64_SVE_VLS_WORDS] = { cpu->sve_vq.map };
-struct kvm_one_reg reg = {
-.id = KVM_REG_ARM64_SVE_VLS,
-.addr = (uint64_t)[0],
-};
 
 assert(cpu->sve_max_vq <= KVM_ARM64_SVE_VQ_MAX);
 
-return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+return kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_VLS, [0]);
 }
 
 #define ARM_CPU_ID_MPIDR   3, 0, 0, 0, 5
@@ -726,19 +722,17 @@ static void kvm_inject_arm_sea(CPUState *c)
 static int kvm_arch_put_fpsimd(CPUState *cs)
 {
 CPUARMState *env = _CPU(cs)->env;
-struct kvm_one_reg reg;
 int i, ret;
 
 for (i = 0; i < 32; i++) {
 uint64_t *q = aa64_vfp_qreg(env, i);
 #if HOST_BIG_ENDIAN
 uint64_t fp_val[2] = { q[1], q[0] };
-reg.addr = (uintptr_t)fp_val;
+ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]),
+fp_val);
 #else
-reg.addr = (uintptr_t)q;
+ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), q);
 #endif
-reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
 if (ret) {
 return ret;
 }
@@ -759,14 +753,11 @@ static int kvm_arch_put_sve(CPUState *cs)
 CPUARMState *env = >env;
 uint64_t tmp[ARM_MAX_VQ * 2];
 uint64_t *r;
-struct kvm_one_reg reg;
 int n, ret;
 
 for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) {
 r = sve_bswap64(tmp, >vfp.zregs[n].d[0], cpu->sve_max_vq * 2);
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r);
 if (ret) {
 return ret;
 }
@@ -775,9 +766,7 @@ static int kvm_arch_put_sve(CPUState *cs)
 for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) {
 r = sve_bswap64(tmp, r = >vfp.pregs[n].p[0],
 DIV_ROUND_UP(cpu->sve_max_vq * 2, 8));
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_PREG(n, 0);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_PREG(n, 0), r);
 if (ret) {
 return ret;
 }
@@ -785,9 +774,7 @@ static int kvm_arch_put_sve(CPUState *cs)
 
 r = sve_bswap64(tmp, >vfp.pregs[FFR_PRED_NUM].p[0],
 DIV_ROUND_UP(cpu->sve_max_vq * 2, 8));
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_FFR(0);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, )

[PATCH v2 3/3] arm/kvm: convert to read_sys_reg64

[Cornelia Huck]

We can use read_sys_reg64 to get the SVE_VLS register instead of
calling GET_ONE_REG directly.

Suggested-by: Gavin Shan 
Signed-off-by: Cornelia Huck 
---
 target/arm/kvm64.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 558c0b88dd69..d40c89a84752 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -500,10 +500,6 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
 .target = -1,
 .features[0] = (1 << KVM_ARM_VCPU_SVE),
 };
-struct kvm_one_reg reg = {
-.id = KVM_REG_ARM64_SVE_VLS,
-.addr = (uint64_t)[0],
-};
 int fdarray[3], ret;
 
 probed = true;
@@ -512,7 +508,7 @@ uint32_t kvm_arm_sve_get_vls(CPUState *cs)
 error_report("failed to create scratch VCPU with SVE enabled");
 abort();
 }
-ret = ioctl(fdarray[2], KVM_GET_ONE_REG, );
+ret = read_sys_reg64(fdarray[2], [0], KVM_REG_ARM64_SVE_VLS);
 kvm_arm_destroy_scratch_host_vcpu(fdarray);
 if (ret) {
 error_report("failed to get KVM_REG_ARM64_SVE_VLS: %s",
-- 
2.41.0

[PATCH v2 0/3] arm/kvm: use kvm_{get,set}_one_reg

[Cornelia Huck]

I sent this cleanup first... in mid-July (ugh). But better late than never, I 
guess.

>From v1:
- fix buglets (thanks Gavin)
- add patch 3 on top

The kvm_{get,set}_one_reg functions have been around for a very long
time, and using them instead of open-coding the ioctl invocations
saves lines of code, and gives us a tracepoint as well. They cannot
be used by invocations of the ioctl not acting on a CPUState, but
that still leaves a lot of conversions in the target/arm code.

target/mips and target/ppc also have some potential for conversions,
but as I cannot test either (and they are both in 'Odd fixes' anyway),
I left them alone.

Survives some testing on a Mt. Snow.

Cornelia Huck (3):
  arm/kvm: convert to kvm_set_one_reg
  arm/kvm: convert to kvm_get_one_reg
  arm/kvm: convert to read_sys_reg64

 target/arm/kvm.c   |  28 +++---
 target/arm/kvm64.c | 129 -
 2 files changed, 40 insertions(+), 117 deletions(-)

-- 
2.41.0

Re: [PATCH for-8.2 2/2] arm/kvm: convert to kvm_get_one_reg

[Cornelia Huck]

[spooky season is coming up, so time for some thread necromancy!]

On Thu, Jul 27 2023, Cornelia Huck  wrote:

> On Tue, Jul 25 2023, Gavin Shan  wrote:
>
>> On 7/24/23 18:48, Cornelia Huck wrote:
>>> On Mon, Jul 24 2023, Gavin Shan  wrote:
>>>>
>>>> On 7/18/23 21:14, Cornelia Huck wrote:
>>>>> We can neaten the code by switching the callers that work on a
>>>>> CPUstate to the kvm_get_one_reg function.
>>>>>
>>>>> Signed-off-by: Cornelia Huck 
>>>>> ---
>>>>>target/arm/kvm.c   | 15 +++-
>>>>>target/arm/kvm64.c | 57 --
>>>>>2 files changed, 18 insertions(+), 54 deletions(-)
>>>>>
>>>>
>>>> The replacements look good to me. However, I guess it's worty to apply
>>>> the same replacements for target/arm/kvm64.c since we're here?
>>>>
>>>> [gshan@gshan arm]$ pwd
>>>> /home/gshan/sandbox/q/target/arm
>>>> [gshan@gshan arm]$ git grep KVM_GET_ONE_REG
>>>> kvm64.c:err = ioctl(fd, KVM_GET_ONE_REG, );
>>>> kvm64.c:return ioctl(fd, KVM_GET_ONE_REG, );
>>>> kvm64.c:ret = ioctl(fdarray[2], KVM_GET_ONE_REG, );
>>> 
>>> These are the callers that don't work on a CPUState (all in initial
>>> feature discovery IIRC), so they need to stay that way.
>>> 
>>
>> Right, All these ioctl commands are issued when CPUState isn't around. 
>> However, there
>> are two wrappers read_sys_{reg32, reg64}(). The ioctl call in 
>> kvm_arm_sve_get_vls()
>> can be replaced by read_sys_reg64(). I guess it'd better to do this in a 
>> separate
>> patch if you agree.
>
> Yes, we could do that, but I'm not sure how much it adds to the
> code... in any case, I agree that this would be a separate patch.

This series has managed to bubble up to the top of my todo list again,
and I think I'll just go ahead and include that as a separate change on
top.

Re: MAINTAINERS still leaves more files uncovered than I'd like

[Cornelia Huck]

On Fri, Sep 29 2023, Thomas Huth  wrote:

> On 29/09/2023 14.05, Cornelia Huck wrote:
>> On Fri, Sep 29 2023, Markus Armbruster  wrote:
>> 
>>> Where are the remaining unmaintained files now?  Top-scoring
>>> directories, files in sub-directories not counted:
>>>
>>>  $ sed 's,/[^/]*$,/,;s,^[^/]*$,./,' unmaintained-files | sort | uniq -c 
>>> | sort -nr
>>>
>>># directory
>> 
>> (...)
>> 
>>>   40 include/standard-headers/linux/
>> 
>> Given that these are changed via update-linux-headers.sh, we should
>> probably add
>> 
>> F: include/standard-headers/
>> 
>> to the Hosts/LINUX entry? (Some headers in standard-headers are covered
>> in individual sections, but most are not.)
>
> Sounds good, could you please send a patch?

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

[PATCH] MAINTAINERS: add standard-headers to Hosts/LINUX

[Cornelia Huck]

The files in there are updated via update-linux-headers.sh.

Signed-off-by: Cornelia Huck 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 355b1960ce46..95df1f3d8884 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -556,6 +556,7 @@ M: Cornelia Huck 
 M: Paolo Bonzini 
 S: Maintained
 F: linux-headers/
+F: include/standard-headers/
 F: scripts/update-linux-headers.sh
 
 POSIX
-- 
2.41.0

Re: MAINTAINERS still leaves more files uncovered than I'd like

[Cornelia Huck]

On Fri, Sep 29 2023, Markus Armbruster  wrote:

> Where are the remaining unmaintained files now?  Top-scoring
> directories, files in sub-directories not counted:
>
> $ sed 's,/[^/]*$,/,;s,^[^/]*$,./,' unmaintained-files | sort | uniq -c | 
> sort -nr
>
>   # directory

(...)

>  40 include/standard-headers/linux/

Given that these are changed via update-linux-headers.sh, we should
probably add

F: include/standard-headers/

to the Hosts/LINUX entry? (Some headers in standard-headers are covered
in individual sections, but most are not.)

Re: [virtio-dev] [PATCH] html: add missing enumitem package

[Cornelia Huck]

On Wed, Sep 27 2023, "Michael S. Tsirkin"  wrote:

> makediffhtml.sh currently fails with:
>
> ! Missing number, treated as zero.
> 
>\c@*
> l.25850 \begin{enumerate}[label=\alph*
>   .]
> ?
> ! Emergency stop.
> 
>\c@*
> l.25850 \begin{enumerate}[label=\alph*
>   .]
>
> Some web searches turned up suggestions to use enumitem and in fact,
> virtio.tex already does this - but virtio-html.tex doesn't.
>
> Adding \usepackage{enumitem} in virtio-html.tex too fixes the issue.
>
> Signed-off-by: Michael S. Tsirkin 
> ---
>  virtio-html.tex | 1 +
>  1 file changed, 1 insertion(+)
>

Oops, should read my mail before pushing. I'll use this one instead.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

[Cornelia Huck]

On Wed, Sep 27 2023, Halil Pasic  wrote:

> On Wed, 27 Sep 2023 12:08:43 +0200
> Cornelia Huck  wrote:
>
>> > On the other hand virtio_airq_handler() calls vring_interrupt() with
>> > interrupts enabled. (While vring_interrupt() is called in a (read)
>> > critical section in virtio_airq_handler() we use read_lock() and
>> > not read_lock_irqsave() to grab the lock. Whether that is correct in
>> > it self (i.e. disregarding the crypto problem) or not I'm not sure right
>> > now. Will think some more about it tomorrow.) If the way to go forward
>> > is disabling interrupts in virtio-ccw before vring_interrupt() is
>> > called, I would be glad to spin a patch for that.  
>> 
>> virtio_airq_handler() is supposed to be an interrupt handler for an
>> adapter interrupt -- as such I would expect it to always run with
>> interrupts disabled (and I'd expect vring_interrupt() to be called
>> with interrupts disabled as well; if that's not the case, I think it
>> would need to run asynchronously.) At least that was my understanding at
>> the time I wrote the code.
>
> Thanks Connie! I don't quite understand what do you mean by "run with
> interrupts disabled" in this context.
>
> Do you mean that if I were to add the following warning:
>
> diff --git a/drivers/s390/virtio/virtio_ccw.c 
> b/drivers/s390/virtio/virtio_ccw.c
> index ac67576301bf..2a9c73f5964f 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -211,6 +211,8 @@ static void virtio_airq_handler(struct airq_struct *airq,
> struct airq_info *info = container_of(airq, struct airq_info, airq);
> unsigned long ai;
>  
> +   WARN_ONCE(in_irq(), "irqs are ought to be disabled but are not\n");
> +
> inc_irq_stat(IRQIO_VAI);
>
> it would/should never trigger, or do you mean something different?
>
> If yes, does that mean that you would expect the common airq code (i.e. 
> something
> like do_airq_interrupt()) to disable interrupts, or call 
> virtio_airq_handler()?
> asynchronously sort of as a bottom half (my understanding of bottom halves is 
> currently
> not complete).
>
> If no what do you actually mean?

My understanding (at the time) was that we're coming from the low-level
interrupt handler (which disables interrupts via the NEW PSW);
interrupts will be re-enabled once the basic processing is done. This
might no longer be the case, but I currently don't have the time to dig
into the code -- it has been some time.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] crypto: virtio-crypto: call finalize with bh disabled

[Cornelia Huck]

On Tue, Sep 26 2023, Halil Pasic  wrote:

> [..]
>> --- a/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
>> +++ b/drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
>> @@ -61,8 +61,9 @@ static void virtio_crypto_akcipher_finalize_req(
>>  vc_akcipher_req->src_buf = NULL;
>>  vc_akcipher_req->dst_buf = NULL;
>>  virtcrypto_clear_request(_akcipher_req->base);
>> -
>> +local_bh_disable();
>>  crypto_finalize_akcipher_request(vc_akcipher_req->base.dataq->engine, 
>> req, err);
>> +local_bh_enable();
>
> Thanks Gonglei!
>
> I did this a quick spin, and it does not seem to be sufficient on s390x.
> Which does not come as a surprise to me, because 
>
> #define lockdep_assert_in_softirq() \
> do {\
> WARN_ON_ONCE(__lockdep_enabled  &&  \
>  (!in_softirq() || in_irq() || in_nmi()));  \
> } while (0)
>
> will still warn because  in_irq() still evaluates to true (your patch
> addresses the !in_softirq() part).
>
> I don't have any results on x86 yet. My current understanding is that the
> virtio-pci transport code disables interrupts locally somewhere in the
> call chain (actually in vp_vring_interrupt() via spin_lock_irqsave())
> and then x86 would be fine. But I will get that verified.
>
> On the other hand virtio_airq_handler() calls vring_interrupt() with
> interrupts enabled. (While vring_interrupt() is called in a (read)
> critical section in virtio_airq_handler() we use read_lock() and
> not read_lock_irqsave() to grab the lock. Whether that is correct in
> it self (i.e. disregarding the crypto problem) or not I'm not sure right
> now. Will think some more about it tomorrow.) If the way to go forward
> is disabling interrupts in virtio-ccw before vring_interrupt() is
> called, I would be glad to spin a patch for that.

virtio_airq_handler() is supposed to be an interrupt handler for an
adapter interrupt -- as such I would expect it to always run with
interrupts disabled (and I'd expect vring_interrupt() to be called
with interrupts disabled as well; if that's not the case, I think it
would need to run asynchronously.) At least that was my understanding at
the time I wrote the code.

>
> Copying Conny, as she may have an opinion on this (if I'm not wrong she
> authored that code).
>
> Regards,
> Halil

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[virtio-dev] [PATCH virtio-1.3] update acknowledgements for 1.3

[Cornelia Huck]

Move some names to the section for previous versions, add names of new
contributors, etc.

Signed-off-by: Cornelia Huck 
---
 acknowledgements.tex | 124 ++-
 1 file changed, 75 insertions(+), 49 deletions(-)

diff --git a/acknowledgements.tex b/acknowledgements.tex
index 1c0a155143bf..39e80d4391d7 100644
--- a/acknowledgements.tex
+++ b/acknowledgements.tex
@@ -6,82 +6,65 @@ \chapter{Acknowledgements}\label{chap:Acknowledgements}
 The following individuals have participated in the creation of this 
specification and are gratefully acknowledged:
 
 \subsection*{Participants}
-Alexander Duyck, Intel \newline
-Alex Bennée, Linaro \newline
+Alvaro Karsz, SolidRun \newline
 Anton Yakovlev, OpenSynergy \newline
-Arseny Krasnov, Kaspersky Lab \newline
 Cornelia Huck, Red Hat \newline
+David Edmondson, Oracle \newline
 David Hildenbrand, Red Hat \newline
-David Stevens, Chromium \newline
-Dr. David Alan Gilbert, Red Hat \newline
+Dmitry Fomichev, Western Digital \newline
+Dust Li, Alibaba \newline
 Enrico Granata, Google \newline
-Eugenio Pérez, Red Hat \newline
-Felipe Franciosi, Nutanix \newline
-Gaetan Harter, OpenSynergy \newline
-Gerd Hoffmann, Red Hat \newline
-Gurchetan Singh, Chromium \newline
+Haixu Cui, Quic Inc \newline
 Halil Pasic, IBM \newline
-Hao Chen, Google \newline
-Huang Yang, Intel \newline
+Heng Qi, Alibaba \newline
+Hrishivarya Bhageeradhan, OpenSynergy \newline
 Jan Kiszka, Siemens \newline
-Jean-Philippe Brucker, Linaro \newline
-Jiang Wang, Bytedance \newline
-Jie Deng, Intel \newline
-Joel Nider, Individual \newline
-Johannes Berg, Intel \newline
-Junji Wei, Bytedance \newline
-Keiichi Watanabe, Chromium \newline
-Marcel Holtmann, Individual \newline
-Max Gurtovoy, Nvidia \newline
+Jiri Pirko, Nvidia \newline
+Laura Loghin, Amazon \newline
+Lei He, Bytedance \newline
+Lingshan Zhu, Intel \newline
+Matti Moell, OpenSynergy \newline
 Michael S. Tsirkin, Red Hat \newline
-Nikos Dragazis, Arrikto \newline
-Pankaj Gupta, Red Hat \newline
-Paolo Bonzini, Red Hat \newline
+Mihai Carabas, Oracle \newline
 Parav Pandit, Nvidia \newline
-Peter Hilber, OpenSynergy \newline
-Petre Eftime, Amazon \newline
-Philipp Hahn, Univention \newline
-Rob Bradford, Intel \newline
-Stefan Fritsch, Individual \newline
+Ran Koren, Nvidia \newline
+Satananda Burla, Marvell \newline
+Shahaf Shuler, Nvidia \newline
+Si-Wei Liu, Oracle \newline
 Stefan Hajnoczi, Red Hat \newline
 Stefano Garzarella, Red Hat \newline
-Taylor Stark, Microsoft \newline
-Tiwei Bie, Intel \newline
-Viresh Kumar, Linaro \newline
-Vitaly Mireyno, Marvell \newline
 Xuan Zhuo, Alibaba \newline
-Yadong Qi, Intel \newline
-Yoni Bettan, Red Hat \newline
 Yuri Benditovich, Red Hat / Daynix \newline
+Zhenwei Pi, Bytedance \newline
 
 The following non-members have provided valuable feedback on this
 specification and are gratefully acknowledged:
 
 \subsection*{Reviewers}
-Christophe de Dinechin, Red Hat \newline
-Gil Savir, Intel \newline
-Ruchika Gupta, Linaro \newline
-Arnd Bergmann, Individual \newline
-Bing Zhu, Intel \newline
-Eduardo Habkost, Red Hat \newline
-Eric Auger, Red Hat \newline
-Jason Wang, Red Hat \newline
-Jens Freimann, Red Hat \newline
-Kevin Tian, Intel \newline
-Linus Walleij, Linaro \newline
-Matti Möll, OpenSynergy \newline
-Tomas Winkler, Intel \newline
-Yang Huang, Intel \newline
+Damien Le Moal, Western Digital \newline
+Hans Holmberg, Western Digital \newline
+Hans Zhang, Alibaba \newline
+He Rongguang, Alibaba \newline
+Helin Guo, Alibaba \newline
+Jiang Liu, Alibaba \newline
+Matias Bjørling, Western Digital \newline
+Max Gurtovoy, Nvidia \newline
+Niklas Cassel, Western Digital \newline
+Tony Lu, Alibaba \newline
 
 
 The following individuals have participated in the creation of
 previous versions of this specification and are gratefully acknowledged:
 
 \subsection*{Participants}
+Alexander Duyck, Intel \newline
+Alex Bennée, Linaro \newline
 Allen Chia, Oracle \newline
 Amit Shah, Red Hat \newline
 Amos Kong, Red Hat \newline
 Anthony Liguori,   IBM \newline
+Anton Yakovlev, OpenSynergy \newline
+Arseny Krasnov, Kaspersky Lab \newline
 Bruce Rogers, SUSE \newline
 Bryan Venteicher,  NetApp  \newline
 Chandra Thyamagondlu, Xilinx   \newline
@@ -90,46 +73,78 @@ \subsection*{Participants}
 Cunming, Liang, Intel  \newline
 Damjan, Marion, Cisco  \newline
 Daniel Kiper,  Oracle  \newline
+David Hildenbrand, Red Hat \newline
+David Stevens, Chromium \newline
+Dr. David Alan Gilbert, Red Hat \newline
+Enrico Granata, Google \newline
+Eugenio Pérez, Red Hat \newline
 Fang Chen, Huawei  \newline
 Fang You, Huawei   \newline
+Felipe Franciosi, Nutanix \newline
+Gaetan Harter, OpenSynergy \newline
 Geoff Brown,   M2Mi\newline
 Gerd Hoffmann, Red Hat \newline
 Gershon Janssen,   Individual Member   \newline
 Grant Likely, ARM  \newline
+Gurchetan Singh, Chromium \newline
 Haggai Eran,   Mellanox\newline
 Halil Pasic

[plasma-systemmonitor] [Bug 473139] sensor configuration not possible, trying to select a sensors shows weird selection, most sensors do not work

[Marco Huck]

https://bugs.kde.org/show_bug.cgi?id=473139

Marco Huck  changed:

   What|Removed |Added

 Resolution|--- |DUPLICATE
 Status|REPORTED|RESOLVED

--- Comment #2 from Marco Huck  ---


*** This bug has been marked as a duplicate of bug 461070 ***

-- 
You are receiving this mail because:
You are watching all bug changes.

[plasma-systemmonitor] [Bug 461070] sensor list in system monitor are changed every time and not correct with C locale

[Marco Huck]

https://bugs.kde.org/show_bug.cgi?id=461070

Marco Huck  changed:

   What|Removed |Added

 CC||don.mar...@gmx.net

--- Comment #15 from Marco Huck  ---
*** Bug 473139 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are watching all bug changes.

[plasma-systemmonitor] [Bug 473139] sensor configuration not possible, trying to select a sensors shows weird selection, most sensors do not work

[Marco Huck]

https://bugs.kde.org/show_bug.cgi?id=473139

--- Comment #1 from Marco Huck  ---
I think this is a duplicate of https://bugs.kde.org/show_bug.cgi?id=461070

-- 
You are receiving this mail because:
You are watching all bug changes.

[plasma-systemmonitor] [Bug 473139] New: sensor configuration not possible, trying to select a sensors shows weird selection, most sensors do not work

[Marco Huck]

https://bugs.kde.org/show_bug.cgi?id=473139

Bug ID: 473139
   Summary: sensor configuration not possible, trying to select a
sensors shows weird selection, most sensors do not
work
Classification: Applications
   Product: plasma-systemmonitor
   Version: 5.27.6
  Platform: Gentoo Packages
OS: Linux
Status: REPORTED
  Severity: normal
  Priority: NOR
 Component: general
  Assignee: ksysguard-b...@kde.org
  Reporter: don.mar...@gmx.net
CC: ahiems...@heimr.nl, plasma-b...@kde.org
  Target Milestone: ---

SUMMARY

STEPS TO REPRODUCE
1. Open KDE System Monitor
2. try to select sensors
3. 

OBSERVED RESULT
Selection of sensors is totally wrong, CPU sensors listed under GPU category
and so on. CPU temp sensors do not work at all (they do work on console with
lm_sensors and sensors commands)

EXPECTED RESULT
select different system sensors, show correct values

SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: Gentoo Linux 2.13
(available in About System)
KDE Plasma Version: 5.27.6
KDE Frameworks Version: 5.108.0
Qt Version: 5.15.10

ADDITIONAL INFORMATION
Kernel version 6.4.8-gentoo (64-bit)

-- 
You are receiving this mail because:
You are watching all bug changes.

Re: [PATCH for-8.2] hw/s390x/s390-virtio-ccw: Remove superfluous code to set the NIC model

[Cornelia Huck]

On Fri, Aug 04 2023, Thomas Huth  wrote:

> The check for nd->model being NULL was originally required, but in
> commit e11f463295d95aba ("s390x/virtio: use qemu_check_nic_model()")
> the corresponding code had been replaced by a call to the function
> qemu_check_nic_model() - and this in turn calls qemu_find_nic_model()
> which contains the same check for nd->model being NULL again. So we
> can remove this from the calling site now.
>
> Signed-off-by: Thomas Huth 
> ---
>  hw/s390x/s390-virtio-ccw.c | 4 
>  1 file changed, 4 deletions(-)

Reviewed-by: Cornelia Huck 

[virtio-dev] [virtio 1.3] Change freeze has started

[Cornelia Huck]

As outlined in
https://lists.oasis-open.org/archives/virtio/202304/msg00036.html ff.,
as part of the release process for the 1.3 version of the virtio spec,
we have now entered change freeze. This means that the only changes to
the main virtio branch will now be those needed for the 1.3 release
(administrative and editorial.) We hope to have something ready for vote
in August.

For any changes for the next virtio release, please target the
virtio-1.4 branch, which will eventually merge back into the main
development branch once 1.3 has been released. (This next release might
not be called 1.4, but that should be inconsequential here.)

virtio 1.3 timeline:

Development

July 3rd: feature freeze (github issue
and change on list)

August 2nd: change freeze (all voted
upon)

virtio-next development branch opens  < we are here

August: prepare draft

August 31st (latest): draft voted upon by TC

September: public review period

October: addtl public review period, if needed

October (November): 1.3 released

merge virtio-next development branch


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

Re: [PATCH for-8.2 2/2] arm/kvm: convert to kvm_get_one_reg

[Cornelia Huck]

On Tue, Jul 25 2023, Gavin Shan  wrote:

> On 7/24/23 18:48, Cornelia Huck wrote:
>> On Mon, Jul 24 2023, Gavin Shan  wrote:
>>>
>>> On 7/18/23 21:14, Cornelia Huck wrote:
>>>> We can neaten the code by switching the callers that work on a
>>>> CPUstate to the kvm_get_one_reg function.
>>>>
>>>> Signed-off-by: Cornelia Huck 
>>>> ---
>>>>target/arm/kvm.c   | 15 +++-
>>>>target/arm/kvm64.c | 57 --
>>>>2 files changed, 18 insertions(+), 54 deletions(-)
>>>>
>>>
>>> The replacements look good to me. However, I guess it's worty to apply
>>> the same replacements for target/arm/kvm64.c since we're here?
>>>
>>> [gshan@gshan arm]$ pwd
>>> /home/gshan/sandbox/q/target/arm
>>> [gshan@gshan arm]$ git grep KVM_GET_ONE_REG
>>> kvm64.c:err = ioctl(fd, KVM_GET_ONE_REG, );
>>> kvm64.c:return ioctl(fd, KVM_GET_ONE_REG, );
>>> kvm64.c:ret = ioctl(fdarray[2], KVM_GET_ONE_REG, );
>> 
>> These are the callers that don't work on a CPUState (all in initial
>> feature discovery IIRC), so they need to stay that way.
>> 
>
> Right, All these ioctl commands are issued when CPUState isn't around. 
> However, there
> are two wrappers read_sys_{reg32, reg64}(). The ioctl call in 
> kvm_arm_sve_get_vls()
> can be replaced by read_sys_reg64(). I guess it'd better to do this in a 
> separate
> patch if you agree.

Yes, we could do that, but I'm not sure how much it adds to the
code... in any case, I agree that this would be a separate patch.

[hpx-commits] [STEllAR-GROUP/hpx] 967ff1: Updating APEX default version to v2.6.3

[Kevin Huck]

  Branch: refs/heads/release-1.9.X
  Home:   https://github.com/STEllAR-GROUP/hpx
  Commit: 967ff1aa3f7338e5a014a49f8f0a94f0cf778e0d
  
https://github.com/STEllAR-GROUP/hpx/commit/967ff1aa3f7338e5a014a49f8f0a94f0cf778e0d
  Author: Kevin Huck 
  Date:   2023-07-26 (Wed, 26 Jul 2023)

  Changed paths:
M CMakeLists.txt

  Log Message:
  ---
  Updating APEX default version to v2.6.3

Updating APEX default version to v2.6.3


___
hpx-commits mailing list
hpx-commits@mail.cct.lsu.edu
https://mail.cct.lsu.edu/mailman/listinfo/hpx-commits

Re: [PATCH for-8.2 2/2] arm/kvm: convert to kvm_get_one_reg

[Cornelia Huck]

On Mon, Jul 24 2023, Gavin Shan  wrote:

> Hi Connie,
>
> On 7/18/23 21:14, Cornelia Huck wrote:
>> We can neaten the code by switching the callers that work on a
>> CPUstate to the kvm_get_one_reg function.
>> 
>> Signed-off-by: Cornelia Huck 
>> ---
>>   target/arm/kvm.c   | 15 +++-
>>   target/arm/kvm64.c | 57 --
>>   2 files changed, 18 insertions(+), 54 deletions(-)
>> 
>
> The replacements look good to me. However, I guess it's worty to apply
> the same replacements for target/arm/kvm64.c since we're here?
>
> [gshan@gshan arm]$ pwd
> /home/gshan/sandbox/q/target/arm
> [gshan@gshan arm]$ git grep KVM_GET_ONE_REG
> kvm64.c:err = ioctl(fd, KVM_GET_ONE_REG, );
> kvm64.c:return ioctl(fd, KVM_GET_ONE_REG, );
> kvm64.c:ret = ioctl(fdarray[2], KVM_GET_ONE_REG, );

These are the callers that don't work on a CPUState (all in initial
feature discovery IIRC), so they need to stay that way.

Re: [PATCH for-8.2 1/2] arm/kvm: convert to kvm_set_one_reg

[Cornelia Huck]

On Mon, Jul 24 2023, Gavin Shan  wrote:

> Hi Connie,
>
> On 7/18/23 21:14, Cornelia Huck wrote:
>> We can neaten the code by switching to the kvm_set_one_reg function.
>> 
>> Signed-off-by: Cornelia Huck 
>> ---
>>   target/arm/kvm.c   | 13 +++--
>>   target/arm/kvm64.c | 66 +-
>>   2 files changed, 21 insertions(+), 58 deletions(-)
>> 
>
> Some wrong replacements to be fixed in kvm_arch_put_fpsimd() as below.
> Apart from that, LGTM:
>
> Reviewed-by: Gavin Shan 
> @@ -725,19 +721,17 @@ static void kvm_inject_arm_sea(CPUState *c)
>>   static int kvm_arch_put_fpsimd(CPUState *cs)
>>   {
>>   CPUARMState *env = _CPU(cs)->env;
>> -struct kvm_one_reg reg;
>>   int i, ret;
>>   
>>   for (i = 0; i < 32; i++) {
>>   uint64_t *q = aa64_vfp_qreg(env, i);
>>   #if HOST_BIG_ENDIAN
>>   uint64_t fp_val[2] = { q[1], q[0] };
>> -reg.addr = (uintptr_t)fp_val;
>> +ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]),
>> +_val);
> ^^^
> s/_val/fp_val
>>   #else
>> -reg.addr = (uintptr_t)q;
>> +ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), 
>> );
>   
>  ^^^
>   
> s//q
>   
>  
>>   #endif

Whoops, I thought I had double-checked these...

Re: [virtio-dev] [virtio 1.3] Feature freeze has started

[Cornelia Huck]

On Mon, Jul 03 2023, Cornelia Huck  wrote:

> As outlined in
> https://lists.oasis-open.org/archives/virtio/202304/msg00036.html ff.,
> as part of the release process for the 1.3 version of the virtio spec,
> we have now entered feature freeze. This means:
>
> ***
> By July 1st[3rd], all not yet integrated features that are targeting 1.3 need
> to have at least a github issue open that is pointing to a patch (set)
> on the list.
> ***
>
> AFAICS, the list of features matching these criteria is the following:
>
> https://github.com/oasis-tcs/virtio-spec/issues/173 ("virtio-net:
> support inner header hash")
>   * this looks very close, with voting likely to start in the next
> couple of days
>
> https://github.com/oasis-tcs/virtio-spec/issues/167 ("Support
> transitional device for PCIe VF")
>   * I have not followed that one in detail, but it has been stated that
> this one should also be ready soon -- can people please confirm that
> this will be ready for inclusion in July?
>
> https://github.com/oasis-tcs/virtio-spec/issues/170 ("The size of
> virtio_net_hdr struct are wrong when VIRTIO_NET_HASH_REPORT feature is
> negotiated.")
>  * Can someone please check the status of this? Looks like a patch is
>available, but no progress for a long time? Is this maybe interacting
>with the inner header hash feature?
>
> Any others that I have missed?

Last call for 1.3. The first two are done now, the third one has not
seen any action, so if anything, it's going to be post-1.3.

I think we're good (and can start preparing for the next steps).

>
> I'd like to wrap this up in July; not a real problem if we spill over
> a couple of days into August, but I'd really like to avoid delaying the
> process for too much (it will take long enough already.)
>
> virtio 1.3 timeline:
>
> Development
>
> July 3rd: feature freeze (github issue   <-- we are here
> and change on list)
>
> August 1st: change freeze (all voted
> upon)
>
> virtio-next development branch opens
>
> August: prepare draft
>
> August 31st (latest): draft voted upon by TC
>
> September: public review period
>
> October: addtl public review period, if needed
>
> October (November): 1.3 released
>
> merge virtio-next development branch
>
>
> -
> To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: 1.3 and branching

[Cornelia Huck]

On Fri, Jul 21 2023, "Michael S. Tsirkin"  wrote:

> On Fri, Jul 21, 2023 at 02:24:57PM +0200, Cornelia Huck wrote:
>> On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:
>> 
>> > On Wed, Jul 12, 2023 at 02:24:32PM +0200, Cornelia Huck wrote:
>> >> On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:
>> >> > Yes we were supposed to freeze for 1.3. This change can be merged on a
>> >> > main branch after 1.3 forks and is under review.
>> >> 
>> >> Nod, that's what I would prefer to do. Being merged on virtio-next
>> >> should be enough for including device/driver implementations.
>> >
>> > Except I'd prefer a v1.3 branch instead of a next branch - adding things
>> > on the branch should be harder, not easier.
>> 
>> Just to clarify: What I had planned for 1.3 (and what we already did for
>> 1.2) is to fork a -next branch, finish 1.3 on the master branch, and
>> then merge -next back into master after we'd be done with 1.3.
>> 
>> I'm not sure what you mean by "adding things on the branch should be
>> harder, not easier" -- I think it is already a bit harder because it is
>> a branch :)
>> 
>> We can of course do a v1.3 branch instead and continue developing on
>> master, but shouldn't we then create branches (glorified tags) for the
>> older releases as well?
>
> Yea, makes sense.
> I think we are all set WRT what we planned to be in 1.3 - right?

I'll send out a "last call" JFTR, but I think we're good.

> Next step is preparing the changelog and packaging it
> all as WD, then voting to approve it as a CSD/CSPRD and start
> public review.
>
> Have time this week? if not I will get to it next week.

Certainly not this week, but one of us can get started with the
changelog next week. Fortunately, it should be shorter than the one for
1.2 :)


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] 1.3 and branching (was: [virtio-dev] Re: [PATCH v12] virtio-net: support device stats)

[Cornelia Huck]

On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:

> On Wed, Jul 12, 2023 at 02:24:32PM +0200, Cornelia Huck wrote:
>> On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:
>> > Yes we were supposed to freeze for 1.3. This change can be merged on a
>> > main branch after 1.3 forks and is under review.
>> 
>> Nod, that's what I would prefer to do. Being merged on virtio-next
>> should be enough for including device/driver implementations.
>
> Except I'd prefer a v1.3 branch instead of a next branch - adding things
> on the branch should be harder, not easier.

Just to clarify: What I had planned for 1.3 (and what we already did for
1.2) is to fork a -next branch, finish 1.3 on the master branch, and
then merge -next back into master after we'd be done with 1.3.

I'm not sure what you mean by "adding things on the branch should be
harder, not easier" -- I think it is already a bit harder because it is
a branch :)

We can of course do a v1.3 branch instead and continue developing on
master, but shouldn't we then create branches (glorified tags) for the
older releases as well?


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[PATCH] hw: Add compat machines for 8.2

[Cornelia Huck]

Add 8.2 machine types for arm/i440fx/m68k/q35/s390x/spapr.

Signed-off-by: Cornelia Huck 
---
 hw/arm/virt.c  |  9 -
 hw/core/machine.c  |  3 +++
 hw/i386/pc.c   |  3 +++
 hw/i386/pc_piix.c  | 16 +---
 hw/i386/pc_q35.c   | 14 --
 hw/m68k/virt.c |  9 -
 hw/ppc/spapr.c | 15 +--
 hw/s390x/s390-virtio-ccw.c | 14 +-
 include/hw/boards.h|  3 +++
 include/hw/i386/pc.h   |  3 +++
 10 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7d9dbc26633a..2a560271b5fc 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3170,10 +3170,17 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
+static void virt_machine_8_2_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(8, 2)
+
 static void virt_machine_8_1_options(MachineClass *mc)
 {
+virt_machine_8_2_options(mc);
+compat_props_add(mc->compat_props, hw_compat_8_1, hw_compat_8_1_len);
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(8, 1)
+DEFINE_VIRT_MACHINE(8, 1)
 
 static void virt_machine_8_0_options(MachineClass *mc)
 {
diff --git a/hw/core/machine.c b/hw/core/machine.c
index f0d35c640184..da699cf4e147 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,9 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-pci.h"
 
+GlobalProperty hw_compat_8_1[] = {};
+const size_t hw_compat_8_1_len = G_N_ELEMENTS(hw_compat_8_1);
+
 GlobalProperty hw_compat_8_0[] = {
 { "migration", "multifd-flush-after-each-section", "on"},
 { TYPE_PCI_DEVICE, "x-pcie-ari-nextfn-1", "on" },
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3109d5e0e035..54838c0c411d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -114,6 +114,9 @@
 { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
 { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
 
+GlobalProperty pc_compat_8_1[] = {};
+const size_t pc_compat_8_1_len = G_N_ELEMENTS(pc_compat_8_1);
+
 GlobalProperty pc_compat_8_0[] = {
 { "virtio-mem", "unplugged-inaccessible", "auto" },
 };
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ac72e8f5bee1..ce1ac9527493 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -504,13 +504,25 @@ static void pc_i440fx_machine_options(MachineClass *m)
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
 }
 
-static void pc_i440fx_8_1_machine_options(MachineClass *m)
+static void pc_i440fx_8_2_machine_options(MachineClass *m)
 {
 pc_i440fx_machine_options(m);
 m->alias = "pc";
 m->is_default = true;
 }
 
+DEFINE_I440FX_MACHINE(v8_2, "pc-i440fx-8.2", NULL,
+  pc_i440fx_8_2_machine_options);
+
+static void pc_i440fx_8_1_machine_options(MachineClass *m)
+{
+pc_i440fx_8_2_machine_options(m);
+m->alias = NULL;
+m->is_default = false;
+compat_props_add(m->compat_props, hw_compat_8_1, hw_compat_8_1_len);
+compat_props_add(m->compat_props, pc_compat_8_1, pc_compat_8_1_len);
+}
+
 DEFINE_I440FX_MACHINE(v8_1, "pc-i440fx-8.1", NULL,
   pc_i440fx_8_1_machine_options);
 
@@ -519,8 +531,6 @@ static void pc_i440fx_8_0_machine_options(MachineClass *m)
 PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
 
 pc_i440fx_8_1_machine_options(m);
-m->alias = NULL;
-m->is_default = false;
 compat_props_add(m->compat_props, hw_compat_8_0, hw_compat_8_0_len);
 compat_props_add(m->compat_props, pc_compat_8_0, pc_compat_8_0_len);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index dc27a9e223a2..37c4814bedf2 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -379,12 +379,23 @@ static void pc_q35_machine_options(MachineClass *m)
 machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
 }
 
-static void pc_q35_8_1_machine_options(MachineClass *m)
+static void pc_q35_8_2_machine_options(MachineClass *m)
 {
 pc_q35_machine_options(m);
 m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v8_2, "pc-q35-8.2", NULL,
+   pc_q35_8_2_machine_options);
+
+static void pc_q35_8_1_machine_options(MachineClass *m)
+{
+pc_q35_8_2_machine_options(m);
+m->alias = NULL;
+compat_props_add(m->compat_props, hw_compat_8_1, hw_compat_8_1_len);
+compat_props_add(m->compat_props, pc_compat_8_1, pc_compat_8_1_len);
+}
+
 DEFINE_Q35_MACHINE(v8_1, "pc-q35-8.1", NULL,
pc_q35_8_1_machine_options);
 
@@ -393,7 +404,6 @@ static void pc_q35_8_0_machine_options(MachineClass *m)
 PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
 
 pc_q35_8_1_machine_options(m);
-m->alias = NULL;
 compat_props_add(m->compat_props

[PATCH for-8.2 1/2] arm/kvm: convert to kvm_set_one_reg

[Cornelia Huck]

We can neaten the code by switching to the kvm_set_one_reg function.

Signed-off-by: Cornelia Huck 
---
 target/arm/kvm.c   | 13 +++--
 target/arm/kvm64.c | 66 +-
 2 files changed, 21 insertions(+), 58 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index b4c7654f4980..cdbffc3c6e0d 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -561,7 +561,6 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
 bool ok = true;
 
 for (i = 0; i < cpu->cpreg_array_len; i++) {
-struct kvm_one_reg r;
 uint64_t regidx = cpu->cpreg_indexes[i];
 uint32_t v32;
 int ret;
@@ -570,19 +569,17 @@ bool write_list_to_kvmstate(ARMCPU *cpu, int level)
 continue;
 }
 
-r.id = regidx;
 switch (regidx & KVM_REG_SIZE_MASK) {
 case KVM_REG_SIZE_U32:
 v32 = cpu->cpreg_values[i];
-r.addr = (uintptr_t)
+ret = kvm_set_one_reg(cs, regidx, );
 break;
 case KVM_REG_SIZE_U64:
-r.addr = (uintptr_t)(cpu->cpreg_values + i);
+ret = kvm_set_one_reg(cs, regidx, cpu->cpreg_values + i);
 break;
 default:
 g_assert_not_reached();
 }
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
 if (ret) {
 /* We might fail for "unknown register" and also for
  * "you tried to set a register which is constant with
@@ -703,17 +700,13 @@ void kvm_arm_get_virtual_time(CPUState *cs)
 void kvm_arm_put_virtual_time(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
-struct kvm_one_reg reg = {
-.id = KVM_REG_ARM_TIMER_CNT,
-.addr = (uintptr_t)>kvm_vtime,
-};
 int ret;
 
 if (!cpu->kvm_vtime_dirty) {
 return;
 }
 
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ret = kvm_set_one_reg(cs, KVM_REG_ARM_TIMER_CNT, >kvm_vtime);
 if (ret) {
 error_report("Failed to set KVM_REG_ARM_TIMER_CNT");
 abort();
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 94bbd9661fd3..b4d02dff5381 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -540,14 +540,10 @@ static int kvm_arm_sve_set_vls(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
 uint64_t vls[KVM_ARM64_SVE_VLS_WORDS] = { cpu->sve_vq.map };
-struct kvm_one_reg reg = {
-.id = KVM_REG_ARM64_SVE_VLS,
-.addr = (uint64_t)[0],
-};
 
 assert(cpu->sve_max_vq <= KVM_ARM64_SVE_VQ_MAX);
 
-return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+return kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_VLS, [0]);
 }
 
 #define ARM_CPU_ID_MPIDR   3, 0, 0, 0, 5
@@ -725,19 +721,17 @@ static void kvm_inject_arm_sea(CPUState *c)
 static int kvm_arch_put_fpsimd(CPUState *cs)
 {
 CPUARMState *env = _CPU(cs)->env;
-struct kvm_one_reg reg;
 int i, ret;
 
 for (i = 0; i < 32; i++) {
 uint64_t *q = aa64_vfp_qreg(env, i);
 #if HOST_BIG_ENDIAN
 uint64_t fp_val[2] = { q[1], q[0] };
-reg.addr = (uintptr_t)fp_val;
+ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]),
+_val);
 #else
-reg.addr = (uintptr_t)q;
+ret = kvm_set_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), );
 #endif
-reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
 if (ret) {
 return ret;
 }
@@ -758,14 +752,11 @@ static int kvm_arch_put_sve(CPUState *cs)
 CPUARMState *env = >env;
 uint64_t tmp[ARM_MAX_VQ * 2];
 uint64_t *r;
-struct kvm_one_reg reg;
 int n, ret;
 
 for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) {
 r = sve_bswap64(tmp, >vfp.zregs[n].d[0], cpu->sve_max_vq * 2);
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r);
 if (ret) {
 return ret;
 }
@@ -774,9 +765,7 @@ static int kvm_arch_put_sve(CPUState *cs)
 for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) {
 r = sve_bswap64(tmp, r = >vfp.pregs[n].p[0],
 DIV_ROUND_UP(cpu->sve_max_vq * 2, 8));
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_PREG(n, 0);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ret = kvm_set_one_reg(cs, KVM_REG_ARM64_SVE_PREG(n, 0), r);
 if (ret) {
 return ret;
 }
@@ -784,9 +773,7 @@ static int kvm_arch_put_sve(CPUState *cs)
 
 r = sve_bswap64(tmp, >vfp.pregs[FFR_PRED_NUM].p[0],
 DIV_ROUND_UP(cpu->sve_max_vq * 2, 8));
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_FFR(0);
-ret = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
+ret = kvm_set_one_

[PATCH for-8.2 2/2] arm/kvm: convert to kvm_get_one_reg

[Cornelia Huck]

We can neaten the code by switching the callers that work on a
CPUstate to the kvm_get_one_reg function.

Signed-off-by: Cornelia Huck 
---
 target/arm/kvm.c   | 15 +++-
 target/arm/kvm64.c | 57 --
 2 files changed, 18 insertions(+), 54 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index cdbffc3c6e0d..4123f6dc9d72 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -525,24 +525,19 @@ bool write_kvmstate_to_list(ARMCPU *cpu)
 bool ok = true;
 
 for (i = 0; i < cpu->cpreg_array_len; i++) {
-struct kvm_one_reg r;
 uint64_t regidx = cpu->cpreg_indexes[i];
 uint32_t v32;
 int ret;
 
-r.id = regidx;
-
 switch (regidx & KVM_REG_SIZE_MASK) {
 case KVM_REG_SIZE_U32:
-r.addr = (uintptr_t)
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, regidx, );
 if (!ret) {
 cpu->cpreg_values[i] = v32;
 }
 break;
 case KVM_REG_SIZE_U64:
-r.addr = (uintptr_t)(cpu->cpreg_values + i);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, regidx, cpu->cpreg_values + i);
 break;
 default:
 g_assert_not_reached();
@@ -678,17 +673,13 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
 void kvm_arm_get_virtual_time(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
-struct kvm_one_reg reg = {
-.id = KVM_REG_ARM_TIMER_CNT,
-.addr = (uintptr_t)>kvm_vtime,
-};
 int ret;
 
 if (cpu->kvm_vtime_dirty) {
 return;
 }
 
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, KVM_REG_ARM_TIMER_CNT, >kvm_vtime);
 if (ret) {
 error_report("Failed to get KVM_REG_ARM_TIMER_CNT");
 abort();
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index b4d02dff5381..66b52d6f8d23 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -908,14 +908,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 static int kvm_arch_get_fpsimd(CPUState *cs)
 {
 CPUARMState *env = _CPU(cs)->env;
-struct kvm_one_reg reg;
 int i, ret;
 
 for (i = 0; i < 32; i++) {
 uint64_t *q = aa64_vfp_qreg(env, i);
-reg.id = AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]);
-reg.addr = (uintptr_t)q;
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, AARCH64_SIMD_CORE_REG(fp_regs.vregs[i]), q);
 if (ret) {
 return ret;
 } else {
@@ -939,15 +936,12 @@ static int kvm_arch_get_sve(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
 CPUARMState *env = >env;
-struct kvm_one_reg reg;
 uint64_t *r;
 int n, ret;
 
 for (n = 0; n < KVM_ARM64_SVE_NUM_ZREGS; ++n) {
 r = >vfp.zregs[n].d[0];
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_ZREG(n, 0);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_ZREG(n, 0), r);
 if (ret) {
 return ret;
 }
@@ -956,9 +950,7 @@ static int kvm_arch_get_sve(CPUState *cs)
 
 for (n = 0; n < KVM_ARM64_SVE_NUM_PREGS; ++n) {
 r = >vfp.pregs[n].p[0];
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_PREG(n, 0);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_PREG(n, 0), r);
 if (ret) {
 return ret;
 }
@@ -966,9 +958,7 @@ static int kvm_arch_get_sve(CPUState *cs)
 }
 
 r = >vfp.pregs[FFR_PRED_NUM].p[0];
-reg.addr = (uintptr_t)r;
-reg.id = KVM_REG_ARM64_SVE_FFR(0);
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, KVM_REG_ARM64_SVE_FFR(0), r);
 if (ret) {
 return ret;
 }
@@ -979,7 +969,6 @@ static int kvm_arch_get_sve(CPUState *cs)
 
 int kvm_arch_get_registers(CPUState *cs)
 {
-struct kvm_one_reg reg;
 uint64_t val;
 unsigned int el;
 uint32_t fpr;
@@ -989,31 +978,24 @@ int kvm_arch_get_registers(CPUState *cs)
 CPUARMState *env = >env;
 
 for (i = 0; i < 31; i++) {
-reg.id = AARCH64_CORE_REG(regs.regs[i]);
-reg.addr = (uintptr_t) >xregs[i];
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.regs[i]),
+  >xregs[i]);
 if (ret) {
 return ret;
 }
 }
 
-reg.id = AARCH64_CORE_REG(regs.sp);
-reg.addr = (uintptr_t) >sp_el[0];
-ret = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+ret = kvm_get_one_reg(cs, AARCH64_CORE_REG(regs.sp), >sp_el[0]);
 if (ret) {
 return ret;
 }
 
-reg.id = AARCH64_CORE_REG(sp_el1);
-reg.addr = (uintptr_t) >sp_el[1];
-ret = kvm_vcpu_ioctl(cs,

[PATCH for-8.2 0/2] arm/kvm: use kvm_{get,set}_one_reg

[Cornelia Huck]

The kvm_{get,set}_one_reg functions have been around for a very long
time, and using them instead of open-coding the ioctl invocations
saves lines of code, and gives us a tracepoint as well. They cannot
be used by invocations of the ioctl not acting on a CPUState, but
that still leaves a lot of conversions in the target/arm code.

target/mips and target/ppc also have some potential for conversions,
but as I cannot test either (and they are both in 'Odd fixes' anyway),
I left them alone.

Survives some testing on a Mt. Snow.

Cornelia Huck (2):
  arm/kvm: convert to kvm_set_one_reg
  arm/kvm: convert to kvm_get_one_reg

 target/arm/kvm.c   |  28 +++
 target/arm/kvm64.c | 123 -
 2 files changed, 39 insertions(+), 112 deletions(-)

-- 
2.41.0

Re: [PATCH 3/3] hw/arm/virt: Support host CPU type only when KVM or HVF is configured

[Cornelia Huck]

On Thu, Jul 13 2023, Gavin Shan  wrote:

> The CPU type 'host-arm-cpu' class won't be registered until KVM or
> HVF is configured in target/arm/cpu64.c. Support the corresponding
> CPU type only when KVM or HVF is configured.
>
> Signed-off-by: Gavin Shan 
> ---
>  hw/arm/virt.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 43d7772ffd..ad28634445 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -217,7 +217,9 @@ static const char *valid_cpu_types[] = {
>  #endif
>  ARM_CPU_TYPE_NAME("cortex-a53"),
>  ARM_CPU_TYPE_NAME("cortex-a57"),
> +#if defined(CONFIG_KVM) || defined(CONFIG_HVF)
>  ARM_CPU_TYPE_NAME("host"),
> +#endif
>  ARM_CPU_TYPE_NAME("max"),
>  NULL
>  };

Doesn't the check in parse_cpu_option() already catch the case where
the "host" cpu model isn't registered? I might be getting lost in the
code flow, though.

[virtio-dev] Re: [virtio-comment] Re: [PATCH 0/4] Short document fixes to inner hash feature

[Cornelia Huck]

On Thu, Jul 13 2023, Cornelia Huck  wrote:

> On Thu, Jul 13 2023, Parav Pandit  wrote:
>
>> This short patches fixes the editing errors that stops the pdf and html
>> generation.
>>
>> These 3 fixes are for the patch [1] for the github issue [2].
>>
>> [1] https://lists.oasis-open.org/archives/virtio-comment/202307/msg00024.html
>> [2] https://github.com/oasis-tcs/virtio-spec/issues/173
>>
>> Patch summary:
>> patch-1 place C code under listing
>> patch-2 avoid hyphen and extra braces
>> patch-3 use table as hyperlink do not work well in C code listing
>> patch-4 refer 'advice' as 'note'
>>
>> Patch 1 to 3 appears to be must in the testing.
>> Patch 4 is not a fix and can be done later if it requires discussion.
>>
>> Parav Pandit (4):
>>   virtio-net: Place C code under listing
>>   virtio-net: Avoid hyphen and extra braces
>>   virtio-net: Use table to describe inner hash to rfc mapping
>>   virtio-net: Use note instead of advice
>>
>>  device-types/net/description.tex | 45 ++--
>>  introduction.tex | 15 +--
>>  2 files changed, 38 insertions(+), 22 deletions(-)
>>
>
> FTR, this is the diff I have locally (I had missed one underscore in the
> references yesterday...); maybe we can make the intra-reference links in
> introdcution.tex a bit nicer, but otherwise, this should be the minimal
> change to make this build:

For the "make it look nicer" part, I think this one would get the job
done; opinions?

diff --git a/introduction.tex b/introduction.tex
index 6f10a94b6fde..1e39a4b2c529 100644
--- a/introduction.tex
+++ b/introduction.tex
@@ -106,8 +106,8 @@ \section{Normative References}\label{sec:Normative 
References}
 Generic Routing Encapsulation. This protocol is only specified for IPv4 
and used as either the payload or delivery protocol.
\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
\phantomsection\label{intro:rfc2890}\textbf{[RFC2890]} &
-Key and Sequence Number Extensions to GRE \ref{intro:rfc2784}. This 
protocol describes extensions by which two fields, Key and
-Sequence Number, can be optionally carried in the GRE Header 
\ref{intro:rfc2784}.
+Key and Sequence Number Extensions to \hyperref[intro:rfc2784]{GRE}. This 
protocol describes extensions by which two fields, Key and
+Sequence Number, can be optionally carried in the 
\hyperref[intro:rfc2784]{GRE} Header.
\newline\url{https://www.rfc-editor.org/rfc/rfc2890}\\
\phantomsection\label{intro:rfc7676}\textbf{[RFC7676]} &
 IPv6 Support for Generic Routing Encapsulation (GRE). This protocol is 
specified for IPv6 and used as either the payload or


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH 2/4] virtio-net: Avoid hyphen and extra braces

[Cornelia Huck]

On Thu, Jul 13 2023, Parav Pandit  wrote:

> Avoid hyphen and replace it with white space like rest of the entries.
> Also avoid unnecessary braces.
> Name RFC as just RFC without special prefix about it.
>
> This likely resolves the html generation errors.
>
> Signed-off-by: Parav Pandit 
> ---
>  device-types/net/description.tex |  2 +-
>  introduction.tex | 15 +++
>  2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/device-types/net/description.tex 
> b/device-types/net/description.tex
> index 6fd4a20..53c811f 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -904,7 +904,7 @@ \subsubsection{Processing of Incoming 
> Packets}\label{sec:Device Types / Network
>  \end{itemize}
>  
>  The per-packet hash calculation can depend on the IP packet type. See
> -\hyperref[intro:IP]{[IP]}, \hyperref[intro:UDP]{[UDP]} and 
> \hyperref[intro:TCP]{[TCP]}.
> +\hyperref[intro:IP]{IP}, \hyperref[intro:UDP]{UDP} and 
> \hyperref[intro:TCP]{TCP}.

I'd keep that as-is (I think it looks nicer with the brackets).

>  
>  \subparagraph{Supported/enabled hash types}
>  \label{sec:Device Types / Network Device / Device Operation / Processing of 
> Incoming Packets / Hash calculation for incoming packets / Supported/enabled 
> hash types}
> diff --git a/introduction.tex b/introduction.tex
> index 81f07a4..028ec17 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -101,26 +101,25 @@ \section{Normative References}\label{sec:Normative 
> References}
>   \phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
>  Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic 
> Cureve Cryptography'', Version 1.0, September 2000.
>   \newline\url{https://www.secg.org/sec1-v2.pdf}\\
> -
> - \phantomsection\label{intro:gre_rfc2784}\textbf{[GRE_rfc2784]} &
> + \phantomsection\label{intro:rfc2784}\textbf{[RFC2784]} &
>  Generic Routing Encapsulation. This protocol is only specified for IPv4 
> and used as either the payload or delivery protocol.
>   \newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> - \phantomsection\label{intro:gre_rfc2890}\textbf{[GRE_rfc2890]} &
> -Key and Sequence Number Extensions to GRE \ref{intro:gre_rfc2784}. This 
> protocol describes extensions by which two fields, Key and
> -Sequence Number, can be optionally carried in the GRE Header 
> \ref{intro:gre_rfc2784}.
> + \phantomsection\label{intro:rfc2890}\textbf{[RFC2890]} &
> +Key and Sequence Number Extensions to GRE \ref{intro:rfc2784}. This 
> protocol describes extensions by which two fields, Key and
> +Sequence Number, can be optionally carried in the GRE Header 
> \ref{intro:rfc2784}.
>   \newline\url{https://www.rfc-editor.org/rfc/rfc2890}\\
> - \phantomsection\label{intro:gre_rfc7676}\textbf{[GRE_rfc7676]} &
> + \phantomsection\label{intro:rfc7676}\textbf{[RFC7676]} &
>  IPv6 Support for Generic Routing Encapsulation (GRE). This protocol is 
> specified for IPv6 and used as either the payload or
>  delivery protocol. Note that this does not change the GRE header format 
> or any behaviors specified by RFC 2784 or RFC 2890.
>   \newline\url{https://datatracker.ietf.org/doc/rfc7676/}\\
> - \phantomsection\label{intro:gre_in_udp_rfc8086}\textbf{[GRE-in-UDP]} &
> + \phantomsection\label{intro:rfc8086}\textbf{[GRE in UDP]} &

Hyphens are ok, I'd keep it.

>  GRE-in-UDP Encapsulation. This specifies a method of encapsulating 
> network protocol packets within GRE and UDP headers.
>  This protocol is specified for IPv4 and IPv6, and used as either the 
> payload or delivery protocol.
>   \newline\url{https://www.rfc-editor.org/rfc/rfc8086}\\
>   \phantomsection\label{intro:vxlan}\textbf{[VXLAN]} &
>  Virtual eXtensible Local Area Network.
>   \newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> - \phantomsection\label{intro:vxlan-gpe}\textbf{[VXLAN-GPE]} &
> + \phantomsection\label{intro:vxlan gpe}\textbf{[VXLAN GPE]} &
>  Generic Protocol Extension for VXLAN. This protocol describes extending 
> Virtual eXtensible Local Area Network (VXLAN) via changes to the VXLAN header.
>   
> \newline\url{https://www.ietf.org/archive/id/draft-ietf-nvo3-vxlan-gpe-12.txt}\\
>   \phantomsection\label{intro:geneve}\textbf{[GENEVE]} &


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH 1/4] virtio-net: Place C code under listing

[Cornelia Huck]

On Thu, Jul 13 2023, Parav Pandit  wrote:

> With extra white space for the define, pdf generation fails.
> Also place the C code under listing.
>
> Signed-off-by: Parav Pandit 
> ---
>  device-types/net/description.tex | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/device-types/net/description.tex 
> b/device-types/net/description.tex
> index 206020d..6fd4a20 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1024,12 +1024,14 @@ \subsubsection{Processing of Incoming 
> Packets}\label{sec:Device Types / Network
>  If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the 
> command
>  VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner 
> header hash.
>  
> +\begin{lstlisting}
>  struct virtnet_hash_tunnel {
>  le32 enabled_tunnel_types;
>  };
>  
>  #define VIRTIO_NET_CTRL_HASH_TUNNEL 7
> - #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
> +#define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0

I think the indentation here is intentional (as in other, similare
sections.)

> +\end{lstlisting}
>  
>  Field \field{enabled_tunnel_types} contains the bitmask of encapsulation 
> types enabled for inner header hash.
>  See \ref{sec:Device Types / Network Device / Device Operation / Processing 
> of Incoming Packets /


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH 0/4] Short document fixes to inner hash feature

[Cornelia Huck]

On Thu, Jul 13 2023, Parav Pandit  wrote:

> This short patches fixes the editing errors that stops the pdf and html
> generation.
>
> These 3 fixes are for the patch [1] for the github issue [2].
>
> [1] https://lists.oasis-open.org/archives/virtio-comment/202307/msg00024.html
> [2] https://github.com/oasis-tcs/virtio-spec/issues/173
>
> Patch summary:
> patch-1 place C code under listing
> patch-2 avoid hyphen and extra braces
> patch-3 use table as hyperlink do not work well in C code listing
> patch-4 refer 'advice' as 'note'
>
> Patch 1 to 3 appears to be must in the testing.
> Patch 4 is not a fix and can be done later if it requires discussion.
>
> Parav Pandit (4):
>   virtio-net: Place C code under listing
>   virtio-net: Avoid hyphen and extra braces
>   virtio-net: Use table to describe inner hash to rfc mapping
>   virtio-net: Use note instead of advice
>
>  device-types/net/description.tex | 45 ++--
>  introduction.tex | 15 +--
>  2 files changed, 38 insertions(+), 22 deletions(-)
>

FTR, this is the diff I have locally (I had missed one underscore in the
references yesterday...); maybe we can make the intra-reference links in
introdcution.tex a bit nicer, but otherwise, this should be the minimal
change to make this build:

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 206020de567d..76585b0dd6d3 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1024,12 +1024,14 @@ \subsubsection{Processing of Incoming 
Packets}\label{sec:Device Types / Network
 If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the 
command
 VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the inner 
header hash.
 
+\begin{lstlisting}
 struct virtnet_hash_tunnel {
 le32 enabled_tunnel_types;
 };
 
 #define VIRTIO_NET_CTRL_HASH_TUNNEL 7
  #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
+\end{lstlisting}
 
 Field \field{enabled_tunnel_types} contains the bitmask of encapsulation types 
enabled for inner header hash.
 See \ref{sec:Device Types / Network Device / Device Operation / Processing of 
Incoming Packets /
@@ -1063,16 +1065,16 @@ \subsubsection{Processing of Incoming 
Packets}\label{sec:Device Types / Network
 Hash calculation for incoming packets / Encapsulation types supported/enabled 
for inner header hash}
 
 Encapsulation types applicable for inner header hash:
-\begin{lstlisting}
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /* 
\hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /* 
\hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /* 
\hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /* 
\hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN   (1 << 4) /* 
\hyperref[intro:vxlan]{[VXLAN]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 5) /* 
\hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE  (1 << 6) /* 
\hyperref[intro:geneve]{[GENEVE]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /* 
\hyperref[intro:ipip]{[IPIP]} */
-#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE   (1 << 8) /* 
\hyperref[intro:nvgre]{[NVGRE]} */
+\begin{lstlisting}[escapechar=|]
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /* 
|\hyperref[intro:rfc2784]{[RFC2784]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /* 
|\hyperref[intro:rfc2890]{[RFC2890]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /* 
|\hyperref[intro:rfc7676]{[RFC7676]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /* 
|\hyperref[intro:rfc8086]{[GRE-in-UDP]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN   (1 << 4) /* 
|\hyperref[intro:vxlan]{[VXLAN]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 5) /* 
|\hyperref[intro:vxlan-gpe]{[VXLAN-GPE]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE  (1 << 6) /* 
|\hyperref[intro:geneve]{[GENEVE]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /* 
|\hyperref[intro:ipip]{[IPIP]}| */
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE   (1 << 8) /* 
|\hyperref[intro:nvgre]{[NVGRE]}| */
 \end{lstlisting}
 
 \subparagraph{Advice}
diff --git a/introduction.tex b/introduction.tex
index 81f07a4fee19..6f10a94b6fde 100644
--- a/introduction.tex
+++ b/introduction.tex
@@ -102,18 +102,18 @@ \section{Normative References}\label{sec:Normative 
References}
 Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve 
Cryptography'', Version 1.0, September 2000.
\newline\url{https://www.secg.org/sec1-v2.pdf}\\
 
-   \phantomsection\label{intro:gre_rfc2784}\textbf{[GRE_rfc2784]} &
+   \phantomsection\label{intro:rfc2784}\textbf{[RFC2784]} &
 Generic Routing Encapsulation. This protocol 

[virtio-dev] RE: [PATCH 3/4] virtio-net: Use table to describe inner hash to rfc mapping

[Cornelia Huck]

On Wed, Jul 12 2023, Parav Pandit  wrote:

>> From: Michael S. Tsirkin 
>> Sent: Wednesday, July 12, 2023 6:41 PM
>> Hmm. escapechar did not work then?
>
>  Forgot to mention, I tried it, it didn't work.

It seemed to work for me (at least in the pdf) -- I'd prefer that one as
a minimal change.


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[hpx-commits] [STEllAR-GROUP/hpx] 8f43bd: Updating APEX default version to v2.6.3

[Kevin Huck]

  Branch: refs/heads/khuck-patch-1
  Home:   https://github.com/STEllAR-GROUP/hpx
  Commit: 8f43bdee001148481ac755967eedee0fc0dd323b
  
https://github.com/STEllAR-GROUP/hpx/commit/8f43bdee001148481ac755967eedee0fc0dd323b
  Author: Kevin Huck 
  Date:   2023-07-12 (Wed, 12 Jul 2023)

  Changed paths:
M CMakeLists.txt

  Log Message:
  ---
  Updating APEX default version to v2.6.3

Updating APEX default version to v2.6.3


___
hpx-commits mailing list
hpx-commits@mail.cct.lsu.edu
https://mail.cct.lsu.edu/mailman/listinfo/hpx-commits

[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash

[Cornelia Huck]

On Wed, Jul 12 2023, Heng Qi  wrote:

> 在 2023/7/12 下午9:25, Cornelia Huck 写道:
>> ...while the changes above give me a fine pdf, the html generated is
>> broken. The problem seems to originate in the normative references that
>> are added in introduction.tex, but I don't see where it goes astray.
>>
>> [401] [402] [403] [404] (./virtio-v1.2-cs01.aux
>> ! Missing \endcsname inserted.
>> 
>> \unhbox
>> l.49 ...ction}Normative References}}{section.1}{}}
>>
>> ?
>> ! Emergency stop.
>> 
>> \unhbox
>> l.49 ...ction}Normative References}}{section.1}{}}
>>
>> (makehtml.sh on master seems to work ok, so it's something in this
>> patch...)
>>
>> Does anyone else manage to spot the problem?
>
> The problem is that underline cannot be added directly in \textbf, we 
> need a little modification:
>
> \textbf{[GRE_rfc2784]} --> \textbf{[GRE\_rfc2784]}
> \textbf{[GRE_rfc2890]} --> \textbf{[GRE\_rfc2890]}
> \textbf{[GRE_rfc7676]} --> \textbf{[GRE\_rfc7676]}
>
> After this modification, I can successfully compile pdf on overleaf, 
> please try this.

I'm afraid that's not the problem (I had tried this and similar changes
without success); the pdf generation is already fine, but html still
fails with this change (or similar).

[Trying on a standard Fedora 37 with a full texlive install...]


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] Re: [PATCH v21] virtio-net: support inner header hash

[Cornelia Huck]

On Wed, Jul 12 2023, Heng Qi  wrote:

> 在 2023/7/12 下午8:42, Cornelia Huck 写道:
>> On Wed, Jul 12 2023, "Michael S. Tsirkin"  wrote:
>>
>>> On Wed, Jul 12, 2023 at 02:22:26PM +0200, Cornelia Huck wrote:
>>>> On Mon, Jul 03 2023, Heng Qi  wrote:
>>>>
>>>> (...)
>>>>
>>>>> +\paragraph{Inner Header Hash}
>>>>> +\label{sec:Device Types / Network Device / Device Operation / Processing 
>>>>> of Incoming Packets / Inner Header Hash}
>>>>> +
>>>>> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the driver can send the 
>>>>> command
>>>>> +VIRTIO_NET_CTRL_HASH_TUNNEL_SET to configure the calculation of the 
>>>>> inner header hash.
>>>>> +
>>>>> +struct virtnet_hash_tunnel {
>>>>> +le32 enabled_tunnel_types;
>>>>> +};
>>>>> +
>>>>> +#define VIRTIO_NET_CTRL_HASH_TUNNEL 7
>>>>> + #define VIRTIO_NET_CTRL_HASH_TUNNEL_SET 0
>>>> This needs to be wrapped in \begin{lstlisting}..\end{lstlisting}, can do
>>>> so when applying.
>>>>
>>>> (...)
>>>>
>>>>> +Encapsulation types applicable for inner header hash:
>>>>> +\begin{lstlisting}
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784(1 << 0) /* 
>>>>> \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890(1 << 1) /* 
>>>>> \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676(1 << 2) /* 
>>>>> \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP (1 << 3) /* 
>>>>> \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN   (1 << 4) /* 
>>>>> \hyperref[intro:vxlan]{[VXLAN]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 5) /* 
>>>>> \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE  (1 << 6) /* 
>>>>> \hyperref[intro:geneve]{[GENEVE]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP(1 << 7) /* 
>>>>> \hyperref[intro:ipip]{[IPIP]} */
>>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE   (1 << 8) /* 
>>>>> \hyperref[intro:nvgre]{[NVGRE]} */
>>>>> +\end{lstlisting}
>>>> I'm afraid this one doesn't come out quite as intended, we'll end up
>>>> with verbatim "\hyperref" text instead of a link. Anyone have a good
>>>> idea on how to fix that?
>>>>
>>>> I'd prefer to push this now with the first issue addressed and to do an
>>>> (editorial) patch on top to deal with the second issue (unless someone
>>>> can come up with a really trivial fix for it, then I can apply that
>>>> straightaway.)
>>> Someone suggested using escapechar:
>>> https://tex.stackexchange.com/questions/314903/inline-links-in-code-listings
>>>
>>> Didn't try.
>> Looks reasonable (and also revealed a typo for VXLAN-GPE). I think I'll
>> go ahead with this one.
>
> Yes. "intro:vxlan_gpe" -> "intro:vxlan-gpe".
>
> May I ask if the fix to these two problems is for me to make fix patches 
> or for you to solve it when editing?

I'd fix it myself while applying, but...

...while the changes above give me a fine pdf, the html generated is
broken. The problem seems to originate in the normative references that
are added in introduction.tex, but I don't see where it goes astray.

[401] [402] [403] [404] (./virtio-v1.2-cs01.aux
! Missing \endcsname inserted.
 
   \unhbox 
l.49 ...ction}Normative References}}{section.1}{}}
  
? 
! Emergency stop.
 
   \unhbox 
l.49 ...ction}Normative References}}{section.1}{}}

(makehtml.sh on master seems to work ok, so it's something in this
patch...)

Does anyone else manage to spot the problem?


-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org