Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Mon, 4 Dec 2023 17:15:06 +0100
From:   Halil Pasic <pasic@linux.ibm.com>
To:     Christian Borntraeger <borntraeger@linux.ibm.com>
Cc:     Tony Krowiak <akrowiak@linux.ibm.com>, linux-s390@vger.kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        jjherne@linux.ibm.com, alex.williamson@redhat.com,
        kwankhede@nvidia.com, frankja@linux.ibm.com,
        imbrenda@linux.ibm.com, david@redhat.com,
        Halil Pasic <pasic@linux.ibm.com>,
        Reinhard Buendgen <BUENDGEN@de.ibm.com>
Subject: Re: [PATCH] s390/vfio-ap: handle response code 01 on queue reset
Message-ID: <20231204171506.42aa687f.pasic@linux.ibm.com>
In-Reply-To: <05cfc382-d01d-4370-b8bb-d3805e957f2e@linux.ibm.com>
References: <20231129143529.260264-1-akrowiak@linux.ibm.com>
        <b43414ef-7aa4-9e5c-a706-41861f0d346c@linux.ibm.com>
        <1f4720d7-93f1-4e38-a3ad-abaf99596e7c@linux.ibm.com>
        <05cfc382-d01d-4370-b8bb-d3805e957f2e@linux.ibm.com>
Organization: IBM
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Mon, 4 Dec 2023 16:16:31 +0100
Christian Borntraeger <borntraeger@linux.ibm.com> wrote:

> Am 04.12.23 um 15:53 schrieb Tony Krowiak:
> > 
> > 
> > On 11/29/23 12:12, Christian Borntraeger wrote:  
> >> Am 29.11.23 um 15:35 schrieb Tony Krowiak:  
> >>> In the current implementation, response code 01 (AP queue number not valid)
> >>> is handled as a default case along with other response codes returned from
> >>> a queue reset operation that are not handled specifically. Barring a bug,
> >>> response code 01 will occur only when a queue has been externally removed
> >>> from the host's AP configuration; nn this case, the queue must
> >>> be reset by the machine in order to avoid leaking crypto data if/when the
> >>> queue is returned to the host's configuration. The response code 01 case
> >>> will be handled specifically by logging a WARN message followed by cleaning
> >>> up the IRQ resources.
> >>>  
> >>
> >> To me it looks like this can be triggered by the LPAR admin, correct? So it
> >> is not desireable but possible.
> >> In that case I prefer to not use WARN, maybe use dev_warn or dev_err instead.
> >> WARN can be a disruptive event if panic_on_warn is set.  
> > 
> > Yes, it can be triggered by the LPAR admin. I can't use dev_warn here because we don't have a reference to any device, but I can use pr_warn if that suffices.  
> 
> Ok, please use pr_warn then.

Shouldn't we rather make this an 'info'. I mean we probably do not want
people complaining about this condition. Yes it should be a best practice
to coordinate such things with the guest, and ideally remove the resource
from the guest first. But AFAIU our stack is supposed to be able to
handle something like this. IMHO issuing a warning is excessive measure.
I know Reinhard and Tony probably disagree with the last sentence
though. 

Regards,
Halil