Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge/sound upstream 20200219 #1805

Merged

Conversation

kv2019i
Copy link
Collaborator

@kv2019i kv2019i commented Feb 19, 2020

Upstream merge 20200219. Lots of changes to ASoC and Soundwire, so please test.

MiaoheLin and others added 30 commits February 12, 2020 20:09
Fix some typos in the comments. Also fix coding style.
[Sean Christopherson rewrites the comment of write_fault_to_shadow_pgtable
field in struct kvm_vcpu_arch.]

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Hardcode the EPT page-walk level for L2 to be 4 levels, as KVM's MMU
currently also hardcodes the page walk level for nested EPT to be 4
levels.  The L2 guest is all but guaranteed to soft hang on its first
instruction when L1 is using EPT, as KVM will construct 4-level page
tables and then tell hardware to use 5-level page tables.

Fixes: 855feb6 ("KVM: MMU: Add 5 level EPT & Shadow page table support.")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Define PT_MAX_FULL_LEVELS as PT64_ROOT_MAX_LEVEL, i.e. 5, to fix shadow
paging for 5-level guest page tables.  PT_MAX_FULL_LEVELS is used to
size the arrays that track guest pages table information, i.e. using a
"max levels" of 4 causes KVM to access garbage beyond the end of an
array when querying state for level 5 entries.  E.g. FNAME(gpte_changed)
will read garbage and most likely return %true for a level 5 entry,
soft-hanging the guest because FNAME(fetch) will restart the guest
instead of creating SPTEs because it thinks the guest PTE has changed.

Note, KVM doesn't yet support 5-level nested EPT, so PT_MAX_FULL_LEVELS
gets to stay "4" for the PTTYPE_EPT case.

Fixes: 855feb6 ("KVM: MMU: Add 5 level EPT & Shadow page table support.")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
x86_register enum is not used, let's remove it.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The check cpu->hv_clock.system_time < 0 is redundant since system_time
is a u64 and hence can never be less than zero.  But what was actually
meant is to check that the result is positive, since kernel_ns and
v->kvm->arch.kvmclock_offset are both s64.

Reported-by: Colin King <colin.king@canonical.com>
Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
Addresses-Coverity: ("Macro compares unsigned to 0")
Reviewed-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Add this file to a new kvm/arm index.rst, in order for it to
be shown as part of the virt book.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Despite being an old document, it contains lots of information
that could still be useful.

The document has a nice style with makes easy to convert to
ReST. So, let's convert it to ReST.

This patch does:

	- Use proper markups for titles;
	- Mark and proper indent literal blocks;
	- don't use an 'o' character for lists;
	- other minor changes required for the doc to be parsed.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Instead of pointing for a pre-2.4 and a seaparate patch,
update it to match current upstream, as UML was merged
a long time ago.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Several URLs are pointing to outdated places.

Update the references for the URLs whose contents still exists,
removing the others.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Fix document title to match ReST format
- Convert the table to be properly recognized
- Some indentation fixes to match ReST syntax.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use document title markup;
- Convert tables;
- Add blank lines and adjust indentation.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Fix document title to match ReST format
- Convert the table to be properly recognized
- use proper markups for literal blocks
- Some indentation fixes to match ReST

While here, add an index for kvm devices.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use title markups;
- change indent to match ReST syntax;
- use proper table markups;
- use literal block markups.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use title markups;
- change indent to match ReST syntax;
- use proper table markups;
- use literal block markups.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This document is almost in ReST format. The only thing
needed is to mark a list as such and to add an extra
whitespace.

Yet, let's also use the standard document title markup,
as it makes easier if anyone wants later to add sessions
to it.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use standard markup for document title;
- Adjust indentation and add blank lines as needed;
- use the notes markup;
- mark code blocks as such.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use title markups;
- adjust indentation and add blank lines as needed;
- adjust tables to match ReST accepted formats;
- use :field: markups;
- mark code blocks as such.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use standard title markup;
- adjust lists;
- mark code blocks as such.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use title markups;
- adjust indentation and add blank lines as needed;
- use :field: markups;
- Use cross-references;
- mark code blocks as such.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use title markups;
- adjust indentation and add blank lines as needed;
- adjust tables to match ReST accepted formats;
- use :field: markups.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use title markups;
- adjust indentation and add blank lines as needed;
- adjust tables to match ReST accepted formats;
- mark code blocks as such.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
convert api.txt document to ReST format while trying to keep
its format as close as possible with the authors intent, and
avoid adding uneeded markups.

- Use document title and chapter markups;
- Convert tables;
- Add markups for literal blocks;
- use :field: for field descriptions;
- Add blank lines and adjust indentation

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Add proper markups for titles;
- Adjust whitespaces and blank lines to match ReST
  needs;
- Mark literal blocks as such.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Add a title for the document;
- Adjust whitespaces for it to be properly formatted after
  parsed.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use document title and chapter markups;
- Convert tables;
- Add markups for literal blocks;
- use :field: for field descriptions;
- Add blank lines and adjust indentation

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use document title and chapter markups;
- Add markups for literal blocks;
- use :field: for field descriptions;
- Add blank lines and adjust indentation.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use document title and chapter markups;
- Add markups for tables;
- Add markups for literal blocks;
- Add blank lines and adjust indentation.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This file is almost in ReST format. Just a small set of
changes were needed:

    - Add markups for lists;
    - Add markups for a literal block;
    - Adjust whitespaces.

While here, use the standard markup for the document title.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Use document title and chapter markups;
- Add markups for tables;
- Use list markups;
- Add markups for literal blocks;
- Add blank lines.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This file is almost in ReST format. Just one change was
needed:

    - Add markups for a literal block and change its indentation.

While here, use the standard markup for the document title.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
@kv2019i kv2019i force-pushed the merge/sound-upstream-20200219 branch from b09e5b9 to c73a743 Compare February 19, 2020 11:22
@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 19, 2020

It seems most SOF tests are broken after the upstream changes. Probe and firmware load are ok, but playback/record is not working. On UP2, playback attempt fails to:

[ 107.298913] sof_pcm512x sof_pcm512x: ASoC: find BE for widget PCM0P
[ 107.298916] sof_pcm512x sof_pcm512x: ASoC: try BE : SSP5.OUT
[ 107.298920] sof_pcm512x sof_pcm512x: ASoC: try BE : (not set)
[ 107.298923] sof_pcm512x sof_pcm512x: ASoC: try BE : (not set)
[ 107.298926] sof_pcm512x sof_pcm512x: ASoC: try BE : HDA0.OUT
[ 107.298929] sof_pcm512x sof_pcm512x: ASoC: try BE : HDA1.OUT
[ 107.298933] sof_pcm512x sof_pcm512x: ASoC: try BE : HDA2.OUT
[ 107.298937] sof_pcm512x sof_pcm512x: ASoC: can't get playback BE for PCM0P

Debug ongoing.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 19, 2020

The playback errors happen also with broonie/for-next, so SOF is now broken in upstream. :(

@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 19, 2020

@plbossart Hmm, bisect points to this patch now:

commit dd03907
Author: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Date: Mon Feb 10 12:14:37 2020 +0900
ASoC: soc-pcm: call snd_soc_component_open/close() once

component->driver->open)
ret = component->driver->open(component, substream);

if (ret == 0)
component->opened = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i I think this should go in side the previous if no?

}

int snd_soc_component_open(struct snd_soc_component *component,
struct snd_pcm_substream *substream)
{
int ret = 0;

if (!component->opened &&
if (component->opened[substream->stream] &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be if (!component->opened[substream->stream]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart Heh, with this fixes, playback fails again...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but if I add this negation, then things don't work for me again. So it looks like the open needs to happen the second time this routine is called?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, so the state machine looks very odd here...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i what happens when there are multiple capture/playback streams opened? I feel like the opened state is a bad idea

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranj063 Ack. The substreams can be multiple playback streams, so my first patch worked by accident.

I think only sensible thing is to revert the whole thing. I uploaded the patch now, and will sent to the list in a minute.

@@ -297,39 +297,35 @@ EXPORT_SYMBOL_GPL(snd_soc_component_set_jack);
int snd_soc_component_module_get(struct snd_soc_component *component,
int upon_open)
{
if (component->module)
if (component->module++)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that ref count makes no sense to me (including in the original case). We should only increment in case there's no error, and in addition there are already module refcounts, not sure why we need new ones on top?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart Ok let me remove. I tried first to not touch these, but then it completely broke driver load-reload... I'll try again.

ASoC component open/close and snd_soc_component_module_get/put are
called independently for each component-substream pair, so the logic
in the reverted patch was not sufficient and led to PCM playback and
module unload errors.

Fixes: dd03907 ("ASoC: soc-pcm: call snd_soc_component_open/close() once")
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i force-pushed the merge/sound-upstream-20200219 branch from 2a88d4b to 7114a2a Compare February 19, 2020 18:14
@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 19, 2020

@plbossart @ranj063 Let's not merge this. I'll let this run to get test results, but let's retry upstream merge tomorrow (or when the fix is merged).

@plbossart
Copy link
Member

plbossart commented Feb 19, 2020

@plbossart @ranj063 Let's not merge this. I'll let this run to get test results, but let's retry upstream merge tomorrow (or when the fix is merged).

i need the SoundWire patches + the rest of Morimoto-san, so I'd rather merge and see what happens upstream later.

#1802 currently has 52 patches, it's unreviewable :-(

@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 19, 2020

@plbossart wrote:

i need the SoundWire patches + the rest of Morimoto-san, so I'd rather merge and see what happens upstream later.

Aa, ok. In that case, ok to merge this. Let's see what kind of CI feedback we get. I have not done full local test on this, so if CI doesn't provide sane results, then we have very thin data on how healthy this is (I did basic tests with CFL and APL now).

I can do more tests tomorrow or do a new rebase for 20200220 if this doesn't go in.

@plbossart
Copy link
Member

CI is in a ternary logic mode it seems, it reports an error with BDW but no details available at all.
I've ignored all results this morning. @xiulipan @fredoh9 @wenqingfu FYI, CI cannot be trusted at the moment. I know everyone is trying to do their best but at this point we have to use our own local tests and engineering judgement...

@plbossart
Copy link
Member

SOFCI TEST

@ranj063
Copy link
Collaborator

ranj063 commented Feb 20, 2020

SOFCI TEST

@plbossart are the module load/unload failures because of missing modules in the kmod script?

@xiulipan
Copy link

@plbossart I think some network failure cause the CI failure recently, will check if the network is recovery now.

@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 20, 2020

@ranj063 wrote:

@plbossart are the module load/unload failures because of missing modules in the kmod script?

I think it's the new compress module. We need to add it to the kmod scripts:

https://sof-ci.01.org/linuxpr/PR1805/build3128/devicetest/CML_MANTIS_HDA/check-kmod-load-unload.sh/check-kmod-load-unload.sh.txt
rmmod: ERROR: Module snd_pcm is in use by: snd_compress

FYI @xiulipan

@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 20, 2020

@plbossart @xiulipan @ranj063 My local tests are all fine, tested on SDW, HDA and I2S platforms an no issues fine. CI looks good as well. The kmod error comes from the compress/probes patch, so that should be fine as well.

I'll give you and others a few more hours to test, but when Pierre you come online, let's go ahead and merge this.

@ranj063
Copy link
Collaborator

ranj063 commented Feb 20, 2020

@plbossart @xiulipan @ranj063 My local tests are all fine, tested on SDW, HDA and I2S platforms an no issues fine. CI looks good as well. The kmod error comes from the compress/probes patch, so that should be fine as well.

I'll give you and others a few more hours to test, but when Pierre you come online, let's go ahead and merge this.

@kv2019i so with a couple of different patches from you and @morimoto, how do we go about this? revert and then revisit when one of those patches is accepted?

@plbossart
Copy link
Member

SOFCI TEST

@plbossart are the module load/unload failures because of missing modules in the kmod script?

yes there is a new dependency on snd_compress when probes are enabled. I fixed this and just pushed the fix.

Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's merge

@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 20, 2020

@plbossart wrote:

let's merge

Ack, agreed. @ranj063 The patch I have here is a clean revert, so it should be relatively easy to handle in the next upstream merge. I'll merge...

@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 20, 2020

@plbossart As it is a lot patches, just to double-check, should I use github "Create a merge commit" or "Rebase and merge" for this? I already the merge commit in the branch I pushed, so will it create another one if I choose "Create a merge commit"..?

@plbossart
Copy link
Member

@plbossart As it is a lot patches, just to double-check, should I use github "Create a merge commit" or "Rebase and merge" for this? I already the merge commit in the branch I pushed, so will it create another one if I choose "Create a merge commit"..?

I always use 'Create a merge commit', not sure what happens with the 'rebase' option.

@kv2019i kv2019i merged commit 90c55e8 into thesofproject:topic/sof-dev Feb 20, 2020
@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 20, 2020

@plbossart wrote:

I always use 'Create a merge commit', not sure what happens with the 'rebase' option.

Hmm, are you sure? :) I used that now, but it created a bit weird merge commit with my github account name in the from-field. When I looked at your merges, they look different. So either you have configured github differently, or you use the "rebase-and-merged" after all.

FYI @ranj063

Let me know if there is anything other odd in the merge.

@plbossart
Copy link
Member

things look exactly as my own merges, see e.g. 8c0050f with gitk

@xiulipan
Copy link

@plbossart @kv2019i Do we need to change the kmod patches for the snd_compress or this is some module issue that is already fixed?

@kv2019i
Copy link
Collaborator Author

kv2019i commented Feb 21, 2020

@plbossart @kv2019i Do we need to change the kmod patches for the snd_compress or this is some module issue that is already fixed?

@xiulipan Already done -> thesofproject/sof-test@42b0715

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet