date:20201124

Re: [PATCH for-6.0 2/2] aspeed/smc: Add extra controls to request DMA

2020-11-24 Thread Cédric Le Goater

On 11/25/20 3:49 AM, Joel Stanley wrote:
> On Fri, 20 Nov 2020 at 16:16, Cédric Le Goater  wrote:
>>
>> The controller has a set of hidden bits to request/grant DMA access.
> 
> Do you have the ast2600 datasheet? It describes these bits:
> 
> 31 RW DMA Request
> 
> Write SPIR80 = 0xAEED to set this bit ot '1'.
> And hardware will clear the request to '0' after DMA done, or FW can
> clear to '0' by writing SPIR80 = 0xDEEA.
> 
> 30 RO DMA Grant
> 
> 0: DMA is not allowed to be used. All DMA related control registers
> are not allowed to be written.
> 1: DMA is granted to be used.

I see them now :) They are under the SPI controllers but not under 
the BMC SPI controller where I was looking. May be the datasheet 
was updated now ? 
 
> Do you want to add the magic behavior to your model?

Yes. The Aspeed SPI driver needs them. I think the model can be better.
I will send a v2.

Thanks,

C.

> 
>>
>> Cc: Chin-Ting Kuo 
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/ssi/aspeed_smc.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index e3d5e26058c0..c1557ef5d848 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -127,6 +127,8 @@
>>
>>  /* DMA Control/Status Register */
>>  #define R_DMA_CTRL(0x80 / 4)
>> +#define   DMA_CTRL_REQUEST  (1 << 31)
>> +#define   DMA_CTRL_GRANT(1 << 30)
>>  #define   DMA_CTRL_DELAY_MASK   0xf
>>  #define   DMA_CTRL_DELAY_SHIFT  8
>>  #define   DMA_CTRL_FREQ_MASK0xf
>> @@ -1237,6 +1239,11 @@ static void aspeed_smc_dma_done(AspeedSMCState *s)
>>
>>  static void aspeed_smc_dma_ctrl(AspeedSMCState *s, uint64_t dma_ctrl)
>>  {
>> +if (dma_ctrl & DMA_CTRL_REQUEST) {
>> +s->regs[R_DMA_CTRL] = dma_ctrl | DMA_CTRL_GRANT;
>> +return;
>> +}
>> +
>>  if (!(dma_ctrl & DMA_CTRL_ENABLE)) {
>>  s->regs[R_DMA_CTRL] = dma_ctrl;
>>
>> --
>> 2.26.2
>>

[Bug 1905521] [NEW] assert issue locates in hw/scsi/lsi53c895a.c:624: lsi_do_dma: Assertion `s->current' failed

2020-11-24 Thread Gaoning Pan

Public bug reported:

Hello,

I found an assertion failure in hw/scsi/lsi53c895a.c:624

This was found in latest version 5.2.0.


my reproduced environment is as follows:
Host: ubuntu 18.04
Guest: ubuntu 18.04


QEMU boot command line:
qemu-system-x86_64 -enable-kvm -boot c -m 4G -drive 
format=qcow2,file=./ubuntu.img -nic user,hostfwd=tcp:0.0.0.0:-:22 -display 
none -device lsi53c895a -trace lsi\*

Backtrace is as follows:
#0  0x7f845c6eff47 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:51
#1  0x7f845c6f18b1 in __GI_abort () at abort.c:79
#2  0x7f845c6e142a in __assert_fail_base (fmt=0x7f845c868a38 "%s%s%s:%u: 
%s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x55a1034486a0 
"s->current", file=file@entry=0x55a103448360 "../hw/scsi/lsi53c895a.c", 
line=line@entry=624, function=function@entry=0x55a10344ae60 
<__PRETTY_FUNCTION__.31674> "lsi_do_dma") at assert.c:92
#3  0x7f845c6e14a2 in __GI___assert_fail (assertion=0x55a1034486a0 
"s->current", file=0x55a103448360 "../hw/scsi/lsi53c895a.c", line=624, 
function=0x55a10344ae60 <__PRETTY_FUNCTION__.31674> "lsi_do_dma") at 
assert.c:101
#4  0x55a102049c65 in lsi_do_dma (s=0x6260c100, out=1) at 
../hw/scsi/lsi53c895a.c:624
#5  0x55a102051573 in lsi_execute_script (s=0x6260c100) at 
../hw/scsi/lsi53c895a.c:1250
#6  0x55a10205b05d in lsi_reg_writeb (s=0x6260c100, offset=47, val=178 
'\262') at ../hw/scsi/lsi53c895a.c:1984
#7  0x55a10205fef8 in lsi_io_write (opaque=0x6260c100, addr=47, 
val=178, size=1) at ../hw/scsi/lsi53c895a.c:2146
#8  0x55a102d1b791 in memory_region_write_accessor (mr=0x6260cbe0, 
addr=47, value=0x7f8349dfe2f8, size=1, shift=0, mask=255, attrs=...) at 
../softmmu/memory.c:484
#9  0x55a102d1bba8 in access_with_adjusted_size (addr=47, 
value=0x7f8349dfe2f8, size=1, access_size_min=1, access_size_max=1, 
access_fn=0x55a102d1b4de , mr=0x6260cbe0, 
attrs=...) at ../softmmu/memory.c:545
#10 0x55a102d261ef in memory_region_dispatch_write (mr=0x6260cbe0, 
addr=47, data=178, op=MO_8, attrs=...) at ../softmmu/memory.c:1494
#11 0x55a102b57c3c in flatview_write_continue (fv=0x606ea920, 
addr=49199, attrs=..., ptr=0x7f8449efb000, len=1, addr1=47, l=1, 
mr=0x6260cbe0) at ../softmmu/physmem.c:2767
#12 0x55a102b57f07 in flatview_write (fv=0x606ea920, addr=49199, 
attrs=..., buf=0x7f8449efb000, len=1) at ../softmmu/physmem.c:2807
#13 0x55a102b587cb in address_space_write (as=0x55a105ebca80 
, addr=49199, attrs=..., buf=0x7f8449efb000, len=1) at 
../softmmu/physmem.c:2899
#14 0x55a102b58878 in address_space_rw (as=0x55a105ebca80 
, addr=49199, attrs=..., buf=0x7f8449efb000, len=1, 
is_write=true) at ../softmmu/physmem.c:2909
#15 0x55a102ad4d50 in kvm_handle_io (port=49199, attrs=..., 
data=0x7f8449efb000, direction=1, size=1, count=1) at 
../accel/kvm/kvm-all.c:2283
#16 0x55a102ad6a0f in kvm_cpu_exec (cpu=0x62e00400) at 
../accel/kvm/kvm-all.c:2529
#17 0x55a102c26fbb in kvm_vcpu_thread_fn (arg=0x62e00400) at 
../accel/kvm/kvm-cpus.c:49
#18 0x55a1032c08f8 in qemu_thread_start (args=0x60382780) at 
../util/qemu-thread-posix.c:521
#19 0x7f845caa96db in start_thread (arg=0x7f8349dff700) at 
pthread_create.c:463
#20 0x7f845c7d2a3f in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:95


The poc is attached.

** Affects: qemu
 Importance: Undecided
 Assignee: Gaoning Pan (hades0506)
 Status: New

** Attachment added: "lsi-assert.c"
   
https://bugs.launchpad.net/bugs/1905521/+attachment/5437756/+files/lsi-assert.c

** Changed in: qemu
 Assignee: (unassigned) => Gaoning Pan (hades0506)

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1905521

Title:
  assert issue locates in hw/scsi/lsi53c895a.c:624: lsi_do_dma:
  Assertion `s->current' failed

Status in QEMU:
  New

Bug description:
  Hello,

  I found an assertion failure in hw/scsi/lsi53c895a.c:624

  This was found in latest version 5.2.0.

  
  my reproduced environment is as follows:
  Host: ubuntu 18.04
  Guest: ubuntu 18.04


  QEMU boot command line:
  qemu-system-x86_64 -enable-kvm -boot c -m 4G -drive 
format=qcow2,file=./ubuntu.img -nic user,hostfwd=tcp:0.0.0.0:-:22 -display 
none -device lsi53c895a -trace lsi\*

  Backtrace is as follows:
  #0  0x7f845c6eff47 in __GI_raise (sig=sig@entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x7f845c6f18b1 in __GI_abort () at abort.c:79
  #2  0x7f845c6e142a in __assert_fail_base (fmt=0x7f845c868a38 "%s%s%s:%u: 
%s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x55a1034486a0 
"s->current", file=file@entry=0x55a103448360 "../hw/scsi/lsi53c895a.c", 
line=line@entry=624, function=function@entry=0x55a10344ae60 
<__PRETTY_FUNCTION__.31674> "lsi_do_dma") at assert.c:92
  #3  0x7f845c6e14a2 in __GI___assert_fail (assertion=0x55a103

Re: [RFC PATCH 00/27] vDPA software assisted live migration

2020-11-24 Thread Jason Wang

On 2020/11/21 上午2:50, Eugenio Pérez wrote:

This series enable vDPA software assisted live migration for vhost-net
devices. This is a new method of vhost devices migration: Instead of
relay on vDPA device's dirty logging capability, SW assisted LM
intercepts dataplane, forwarding the descriptors between VM and device.

In this migration mode, qemu offers a new vring to the device to
read and write into, and disable vhost notifiers, processing guest and
vhost notifications in qemu. On used buffer relay, qemu will mark the
dirty memory as with plain virtio-net devices. This way, devices does
not need to have dirty page logging capability.

This series is a POC doing SW LM for vhost-net devices, which already
have dirty page logging capabilities. None of the changes have actual
effect with current devices until last two patches (26 and 27) are
applied, but they can be rebased on top of any other. These checks the
device to meet all requirements, and disable vhost-net devices logging
so migration goes through SW LM. This last patch is not meant to be
applied in the final revision, it is in the series just for testing
purposes.

For use SW assisted LM these vhost-net devices need to be instantiated:
* With IOMMU (iommu_platform=on,ats=on)
* Without event_idx (event_idx=off)

So a question is at what level do we want to implement qemu assisted
live migration. To me it could be done at two levels:

1) generic vhost level which makes it work for both vhost-net/vhost-user
and vhost-vDPA

2) a specific type of vhost

To me, having a generic one looks better but it would be much more
complicated. So what I read from this series is it was a vhost kernel
specific software assisted live migration which is a good start.
Actually it may even have real use case, e.g it can save dirty bitmaps
for guest with large memory. But we need to address the above
limitations first.

So I would like to know what's the reason for mandating iommu platform
and ats? And I think we need to fix case of event idx support.

Just the notification forwarding (with no descriptor relay) can be
achieved with patches 7 and 9, and starting migration. Partial applies
between 13 and 24 will not work while migrating on source, and patch
25 is needed for the destination to resume network activity.

It is based on the ideas of DPDK SW assisted LM, in the series of

Actually we're better than that since there's no need the trick like
hardcoded IOVA for mediated(shadow) virtqueue.

DPDK's https://patchwork.dpdk.org/cover/48370/ .

I notice that you do GPA->VA translations and try to establish a VA->VA
(use VA as IOVA) mapping via device IOTLB. This shortcut should work for
vhost-kernel/user but not vhost-vDPA. The reason is that there's no
guarantee that the whole 64bit address range could be used as IOVA. One
example is that for hardware IOMMU like intel, it usually has 47 or 52
bits of address width.

So we probably need an IOVA allocator that can make sure the IOVA is not
overlapped and fit for [1]. We can probably build the IOVA for guest VA
via memory listeners. Then we have

1) IOVA for GPA
2) IOVA for shadow VQ

And advertise IOVA to VA mapping to vhost.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1b48dc03e575a872404f33b04cd237953c5d7498

Comments are welcome.

Thanks!

Eugenio Pérez (27):
vhost: Add vhost_dev_can_log
vhost: Add device callback in vhost_migration_log
vhost: Move log resize/put to vhost_dev_set_log
vhost: add vhost_kernel_set_vring_enable
vhost: Add hdev->dev.sw_lm_vq_handler
virtio: Add virtio_queue_get_used_notify_split
vhost: Route guest->host notification through qemu
vhost: Add a flag for software assisted Live Migration
vhost: Route host->guest notification through qemu
vhost: Allocate shadow vring
virtio: const-ify all virtio_tswap* functions
virtio: Add virtio_queue_full
vhost: Send buffers to device
virtio: Remove virtio_queue_get_used_notify_split
vhost: Do not invalidate signalled used
virtio: Expose virtqueue_alloc_element
vhost: add vhost_vring_set_notification_rcu
vhost: add vhost_vring_poll_rcu
vhost: add vhost_vring_get_buf_rcu
vhost: Return used buffers
vhost: Add vhost_virtqueue_memory_unmap
vhost: Add vhost_virtqueue_memory_map
vhost: unmap qemu's shadow virtqueues on sw live migration
vhost: iommu changes
vhost: Do not commit vhost used idx on vhost_virtqueue_stop
vhost: Add vhost_hdev_can_sw_lm
vhost: forbid vhost devices logging

Re: [PATCH v2 3/8] qnum: QNumValue type for QNum value literals

2020-11-24 Thread Markus Armbruster

Eduardo Habkost  writes:

> On Tue, Nov 24, 2020 at 04:20:37PM +0100, Markus Armbruster wrote:
>> Eduardo Habkost  writes:
>> 
>> > On Tue, Nov 24, 2020 at 09:49:30AM +0100, Markus Armbruster wrote:
>> >> Eduardo Habkost  writes:
>> >> 
>> >> > On Mon, Nov 23, 2020 at 08:51:27AM +0100, Markus Armbruster wrote:
>> >> >> Eduardo Habkost  writes:
>> >> >> 
>> >> >> > On Fri, Nov 20, 2020 at 06:29:16AM +0100, Markus Armbruster wrote:
>> >> >> [...]
>> >> >> >> When the structure of a data type is to be kept away from its 
>> >> >> >> users, I
>> >> >> >> prefer to keep it out of the public header, so the compiler 
>> >> >> >> enforces the
>> >> >> >> encapsulation.
>> >> >> >
>> >> >> > I prefer that too, except that it is impossible when users of the
>> >> >> > API need the compiler to know the struct size.
>> >> >> 
>> >> >> There are cases where the structure of a data type should be
>> >> >> encapsulated, yet its size must be made known for performance (avoid
>> >> >> dynamic memory allocation and pointer chasing).
>> >> >> 
>> >> >> Need for encapsulation correlates with complex algorithms and data
>> >> >> structures.  The cost of dynamic allocation is often in the noise then.
>> >> >
>> >> > I don't know what we are talking about anymore.  None of this
>> >> > applies to the QNum API, right?
>> >> >
>> >> > QNum/QNumValue are not complex data structures, and the reason we
>> >> > need the compiler to know the size of QNumValue is not related to
>> >> > performance at all.
>> >> 
>> >> We started with the question whether to make QNumValue's members
>> >> private.  We digressed to the question when to make members private.
>> >> So back to the original question.
>> >> 
>> >> > We might still want to discourage users of the QNum API from
>> >> > accessing QNum.u/QNumValue.u directly.  Documenting the field as
>> >> > private is a very easy way to do it.
>> >> 
>> >> It's a complete non-issue.  QNum has been around for years, and we
>> >> haven't had any issues that could've been plausibly avoided by asking
>> >> people to refrain from accessing its members.
>> >> 
>> >> If there was an actual need to keep the members private, I'd move the
>> >> struct out of the header, so the compiler enforces privacy.
>> >
>> > Understood.  There's still a question I'd like to answer, to
>> > decide how the API documentation should look like:
>> >
>> >   Is QNum.u/QNumValue.u required to be part of the API
>> >   documentation?
>> >
>> > If accessing that field directly is not necessary for using the
>> > API, I don't think it should appear in the documentation (because
>> > it would be just noise).
>> 
>> The current patch's comment on QNumValue looks good to me.
>> 
>> Does this answer your question?
>
> The current patch (v3) doesn't address the question.  It doesn't
> include documentation for the field, but doesn't hide it.
> kernel-doc will print a warning on that case.

Do we care?  How many such warnings exist before the patch?  Does this
series add just this one, or more?

Use your judgement, then be ready to explain it :)

1 2 3 >

1 - 100 of 239 matches

Mail list logo