All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: "Alexander.Deucher@amd.com" <Alexander.Deucher@amd.com>,
	alsa-devel@alsa-project.org,
	Sergei Miroshnichenko <s.miroshnichenko@yadro.com>,
	"Christian.Koenig@amd.com" <Christian.Koenig@amd.com>
Subject: Re: Adding movable PCIe BARs support in snd_hda_intel
Date: Wed, 24 Mar 2021 10:53:33 -0400	[thread overview]
Message-ID: <d9bc41d7-bbad-24e3-6ad3-90e6dd8466c4@amd.com> (raw)
In-Reply-To: <s5hv99gan2r.wl-tiwai@suse.de>

Appreciate you investing the effort in helping on this.
I will start to merge it now as it doesn't apply cleanly on my branch.

If I understand correctly your main HW access prevention mechanism during
the PCI prepare-rescan period is by bailing out on IOCTLs with the check
of power state == SNDRV_CTL_POWER_D0 or waiting when a user process closes
it's device file descriptor in patches 2 and 5. For command submission
prevention you use the freeze flag from patch 6.
If I haven't missed anything I don't see how those all protect when
new device is plugged while any of those operations are already in 
flight. What prevents concurrent HW access from an IOCTL already running
and HW suspend and MMIO unampping in rescan_preapre which starts after 
IOCTL began ?

Andrey

On 2021-03-24 6:00 a.m., Takashi Iwai wrote:
> On Tue, 23 Mar 2021 19:25:53 +0100,
> Andrey Grodzovsky wrote:
>>
>>>> This will cover IOCTLs and any
>>>> mmapped accesses i guess. Interrupts we discussed above. What above any
>>>> possible background kernel work going on in dedicated threads or work
>>>> items ? Any pointers there what should be blocked and waited for ?
>>>
>>> An alternative idea would be the analogy of the system suspend /
>>> resume.  That is, we forcibly suspend the devices at first somehow,
>>> and also restricts the further accesses by some way.  Then do remap,
>>
>> But that the point I guess, how you block further accesses without those
>> big locks, during S3 i believe user mode gets suspended before the
>> driver and so you don't need to worry about concurrent IOCTLs when going
>> through suspend sequence
> 
> ALSA core still has some legacy card-level power management code,
> which was introduced many years ago at the time we still managed the
> power state via an extra ioctl (hence working individually from the
> base PM code), and a few pieces are still effective for this kind of
> purposes.  Through a quick glance, a couple of places need band-aids,
> but the rest should work.
> 
> A bit more difficult problem is the floating control API calls.  The
> get/put calls might be still in flight when we perform the PCI
> rescan.  This has to be filtered out additionally.
> 
> Below are a patch series I cooked quickly.  Totally untested, just
> checked the compilation.  The first patch is a fix I'll merge in
> anyway, while the rest are RFC.
> 
> 
> thanks,
> 
> Takashi
> 

  reply	other threads:[~2021-03-24 14:54 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-19 21:22 Adding movable PCIe BARs support in snd_hda_intel Andrey Grodzovsky
2021-03-23 11:23 ` Andrey Grodzovsky
2021-03-23 11:39   ` Takashi Iwai
2021-03-23 14:22     ` Andrey Grodzovsky
2021-03-23 14:54       ` Takashi Iwai
2021-03-23 16:11         ` Andrey Grodzovsky
2021-03-23 16:50           ` Takashi Iwai
2021-03-23 17:08             ` Andrey Grodzovsky
2021-03-23 17:29               ` Takashi Iwai
2021-03-23 18:25                 ` Andrey Grodzovsky
2021-03-24 10:00                   ` Takashi Iwai
2021-03-24 14:53                     ` Andrey Grodzovsky [this message]
2021-03-24 15:06                       ` Takashi Iwai
2021-03-24 15:43                         ` Andrey Grodzovsky
2021-03-24 20:36                           ` Takashi Iwai
2021-03-24 21:43                             ` Andrey Grodzovsky
2021-03-25  6:58                               ` Takashi Iwai
2021-03-25 16:13                                 ` Andrey Grodzovsky
2021-03-25 16:38                                   ` Takashi Iwai
2021-03-26 21:27                                     ` Andrey Grodzovsky
2021-03-27  8:17                                       ` Takashi Iwai
2021-03-29 14:47                                         ` Andrey Grodzovsky
2021-03-29 14:52                                           ` Takashi Iwai
2021-03-29 14:57                                             ` Andrey Grodzovsky
2021-03-24 23:36                             ` [PATCH v2 1/8] ALSA: hda: Re-add dropped snd_poewr_change_state() kernel test robot
2021-03-24 23:36                               ` kernel test robot
2021-03-24 23:57                             ` kernel test robot
2021-03-24 23:57                               ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d9bc41d7-bbad-24e3-6ad3-90e6dd8466c4@amd.com \
    --to=andrey.grodzovsky@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Christian.Koenig@amd.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=s.miroshnichenko@yadro.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.