From: Aneesh Kumar K.V <aneesh.kumar@kernel.org>
To: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: linux-coco@lists.linux.dev, kvmarm@lists.linux.dev,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
dan.j.williams@intel.com, aik@amd.com, lukas@wunner.de,
Samuel Ortiz <sameo@rivosinc.com>,
Xu Yilun <yilun.xu@linux.intel.com>,
Jason Gunthorpe <jgg@ziepe.ca>,
Suzuki K Poulose <Suzuki.Poulose@arm.com>,
Steven Price <steven.price@arm.com>,
Bjorn Helgaas <helgaas@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
Marc Zyngier <maz@kernel.org>, Will Deacon <will@kernel.org>,
Oliver Upton <oliver.upton@linux.dev>
Subject: Re: [PATCH RESEND v2 06/12] coco: host: arm64: Add RMM device communication helpers
Date: Fri, 31 Oct 2025 13:34:33 +0530 [thread overview]
Message-ID: <yq5ams57cx9q.fsf@kernel.org> (raw)
In-Reply-To: <20251030181246.00006328@huawei.com>
Jonathan Cameron <jonathan.cameron@huawei.com> writes:
> On Thu, 30 Oct 2025 21:50:22 +0530
> "Aneesh Kumar K.V" <aneesh.kumar@kernel.org> wrote:
>
>> Jonathan Cameron <jonathan.cameron@huawei.com> writes:
>>
>> > On Mon, 27 Oct 2025 15:25:56 +0530
>> > "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
>> >
>>
>> ...
>>
>> >> +static int __do_dev_communicate(enum dev_comm_type type,
>> >> + struct pci_tsm *tsm, unsigned long error_state)
>> >> +{
>> >> + int ret;
>> >> + int state;
>> >> + struct rmi_dev_comm_enter *io_enter;
>> >> + struct cca_host_pf0_dsc *pf0_dsc = to_cca_pf0_dsc(tsm->dsm_dev);
>> >> +
>> >> + io_enter = &pf0_dsc->comm_data.io_params->enter;
>> >> + io_enter->resp_len = 0;
>> >> + io_enter->status = RMI_DEV_COMM_NONE;
>> >> +
>> >> + ret = ___do_dev_communicate(type, tsm);
>> >
>> > Think up a more meaningful name. Counting _ doesn't make for readable code.
>> >
>>
>> I am not sure about this. What do you think?
>>
>> modified drivers/virt/coco/arm-cca-host/rmi-da.c
>> @@ -188,7 +188,7 @@ static inline gfp_t cache_obj_id_to_gfp_flags(u8 cache_obj_id)
>> return GFP_KERNEL_ACCOUNT;
>> }
>>
>> -static int ___do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm)
>> +static int __do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm)
>> {
>> gfp_t cache_alloc_flags;
>> int ret, nbytes, cp_len;
>> @@ -319,7 +319,7 @@ static int ___do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm)
>> return 0;
>> }
>>
>> -static int __do_dev_communicate(enum dev_comm_type type,
>> +static int do_dev_communicate(enum dev_comm_type type,
>> struct pci_tsm *tsm, unsigned long error_state)
>> {
>> int ret;
>> @@ -331,7 +331,7 @@ static int __do_dev_communicate(enum dev_comm_type type,
>> io_enter->resp_len = 0;
>> io_enter->status = RMI_DEV_COMM_NONE;
>>
>> - ret = ___do_dev_communicate(type, tsm);
>> + ret = __do_dev_communicate(type, tsm);
>> if (ret) {
>> if (type == PDEV_COMMUNICATE)
>> rmi_pdev_abort(virt_to_phys(pf0_dsc->rmm_pdev));
>> @@ -355,14 +355,14 @@ static int __do_dev_communicate(enum dev_comm_type type,
>> return state;
>> }
>>
>> -static int do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm,
>> - unsigned long target_state,
>> - unsigned long error_state)
>> +static int move_dev_to_state(enum dev_comm_type type, struct pci_tsm *tsm,
>
> Naming is always tricky. Not sure why this name is appropriate given it's definitely
> still related to dev_communicate.
>
> Maybe just squash do_dev_communicate and __do_dev_coummnicate.
> Slightly long lines will be the result but not too bad.
> I haven't checked what it ends up as after the whole series though
> so maybe it doesn't work out.
>
> static int do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm,
> unsigned long target_state,
> unsigned long error_state)
> {
>
>
> do {
> int state, ret;
> struct rmi_dev_comm_enter *io_enter;
> struct cca_host_pf0_dsc *pf0_dsc = to_cca_pf0_dsc(tsm->dsm_dev);
>
> io_enter = &pf0_dsc->comm_data.io_params->enter;
> io_enter->resp_len = 0;
> io_enter->status = RMI_DEV_COMM_NONE;
>
> ret = ___do_dev_communicate(type, tsm);
> //renamed
>
> if (ret) {
> if (type == PDEV_COMMUNICATE)
> rmi_pdev_abort(virt_to_phys(pf0_dsc->rmm_pdev));
>
> state = error_state;
> } else {
> /*
> * Some device communication error will transition the
> * device to error state. Report that.
> */
> if (type == PDEV_COMMUNICATE)
> ret = rmi_pdev_get_state(virt_to_phys(pf0_dsc->rmm_pdev),
> (enum rmi_pdev_state *)&state);
> if (ret)
> state = error_state;
> }
>
> if (state == error_state) {
> pci_err(tsm->pdev, "device communication error\n");
> return state;
> }
> if (state == target_state)
> return state;
> } while (1);
> }
> Jonathan
>
I need the existing __do_dev_communicate for a followup patch where the
device communication won't loop till state transition.
Something like
void vdev_fetch_object_work(struct work_struct *work)
{
int state;
struct pci_tsm *tsm;
struct cca_host_pf0_dsc *pf0_dsc;
struct dev_comm_work *setup_work;
struct rmi_dev_comm_enter *io_enter;
setup_work = container_of(work, struct dev_comm_work, work);
tsm = setup_work->tsm;
pf0_dsc = to_cca_pf0_dsc(tsm->dsm_dev);
io_enter = &pf0_dsc->comm_data.io_params->enter;
io_enter->resp_len = 0;
io_enter->status = RMI_DEV_COMM_NONE;
guard(mutex)(&pf0_dsc->object_lock);
*setup_work->cache_offset = 0;
memset(setup_work->cache_buf, 0, setup_work->cache_size);
state = __do_dev_communicate(VDEV_COMMUNICATE, tsm, RMI_VDEV_ERROR);
/* return status through dev_comm_work.cache_cache */
if (state == RMI_VDEV_ERROR)
setup_work->cache_size = 0;
complete(&setup_work->complete);
}
Considering current usage is loop till we reach a specific target state,
how abou the below?
modified drivers/virt/coco/arm-cca-host/rmi-da.c
@@ -188,7 +188,7 @@ static inline gfp_t cache_obj_id_to_gfp_flags(u8 cache_obj_id)
return GFP_KERNEL_ACCOUNT;
}
-static int ___do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm)
+static int _do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm)
{
gfp_t cache_alloc_flags;
int ret, nbytes, cp_len;
@@ -319,7 +319,7 @@ static int ___do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm)
return 0;
}
-static int __do_dev_communicate(enum dev_comm_type type,
+static int do_dev_communicate(enum dev_comm_type type,
struct pci_tsm *tsm, unsigned long error_state)
{
int ret;
@@ -331,7 +331,7 @@ static int __do_dev_communicate(enum dev_comm_type type,
io_enter->resp_len = 0;
io_enter->status = RMI_DEV_COMM_NONE;
- ret = ___do_dev_communicate(type, tsm);
+ ret = _do_dev_communicate(type, tsm);
if (ret) {
if (type == PDEV_COMMUNICATE)
rmi_pdev_abort(virt_to_phys(pf0_dsc->rmm_pdev));
@@ -355,14 +355,14 @@ static int __do_dev_communicate(enum dev_comm_type type,
return state;
}
-static int do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm,
+static int wait_for_dev_state(enum dev_comm_type type, struct pci_tsm *tsm,
unsigned long target_state,
unsigned long error_state)
{
int state;
do {
- state = __do_dev_communicate(type, tsm, error_state);
+ state = do_dev_communicate(type, tsm, error_state);
if (state == target_state || state == error_state)
return state;
@@ -372,9 +372,9 @@ static int do_dev_communicate(enum dev_comm_type type, struct pci_tsm *tsm,
return error_state;
}
-static int do_pdev_communicate(struct pci_tsm *tsm, enum rmi_pdev_state target_state)
+static int wait_for_pdev_state(struct pci_tsm *tsm, enum rmi_pdev_state target_state)
{
- return do_dev_communicate(PDEV_COMMUNICATE, tsm, target_state, RMI_PDEV_ERROR);
+ return wait_for_dev_state(PDEV_COMMUNICATE, tsm, target_state, RMI_PDEV_ERROR);
}
static int parse_certificate_chain(struct pci_tsm *tsm)
@@ -497,7 +497,7 @@ static int pdev_set_public_key(struct pci_tsm *tsm)
virt_to_phys(key_params));
}
-static void pdev_communicate_work(struct work_struct *work)
+static void pdev_state_transition_workfn(struct work_struct *work)
{
unsigned long state;
struct pci_tsm *tsm;
@@ -509,13 +509,13 @@ static void pdev_communicate_work(struct work_struct *work)
pf0_dsc = to_cca_pf0_dsc(tsm->dsm_dev);
guard(mutex)(&pf0_dsc->object_lock);
- state = do_pdev_communicate(tsm, setup_work->target_state);
+ state = wait_for_pdev_state(tsm, setup_work->target_state);
WARN_ON(state != setup_work->target_state);
complete(&setup_work->complete);
}
-static int submit_pdev_comm_work(struct pci_dev *pdev, int target_state)
+static int submit_pdev_state_transition_work(struct pci_dev *pdev, int target_state)
{
int ret;
enum rmi_pdev_state state;
@@ -523,7 +523,7 @@ static int submit_pdev_comm_work(struct pci_dev *pdev, int target_state)
struct cca_host_pf0_dsc *pf0_dsc = to_cca_pf0_dsc(pdev);
struct cca_host_comm_data *comm_data = to_cca_comm_data(pdev);
- INIT_WORK_ONSTACK(&comm_work.work, pdev_communicate_work);
+ INIT_WORK_ONSTACK(&comm_work.work, pdev_state_transition_workfn);
init_completion(&comm_work.complete);
comm_work.tsm = pdev->tsm;
comm_work.target_state = target_state;
@@ -548,7 +548,7 @@ int cca_pdev_ide_setup(struct pci_dev *pdev)
{
int ret;
- ret = submit_pdev_comm_work(pdev, RMI_PDEV_NEEDS_KEY);
+ ret = submit_pdev_state_transition_work(pdev, RMI_PDEV_NEEDS_KEY);
if (ret)
return ret;
/*
@@ -563,7 +563,7 @@ int cca_pdev_ide_setup(struct pci_dev *pdev)
if (ret)
return ret;
- return submit_pdev_comm_work(pdev, RMI_PDEV_READY);
+ return submit_pdev_state_transition_work(pdev, RMI_PDEV_READY);
}
void cca_pdev_stop_and_destroy(struct pci_dev *pdev)
@@ -576,7 +576,7 @@ void cca_pdev_stop_and_destroy(struct pci_dev *pdev)
if (WARN_ON(ret != RMI_SUCCESS))
return;
- submit_pdev_comm_work(pdev, RMI_PDEV_STOPPED);
+ submit_pdev_state_transition_work(pdev, RMI_PDEV_STOPPED);
ret = rmi_pdev_destroy(rmm_pdev_phys);
if (WARN_ON(ret != RMI_SUCCESS))
-aneesh
next prev parent reply other threads:[~2025-10-31 8:04 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-27 9:55 [PATCH RESEND v2 00/12] coc: tsm: Implement ->connect()/->disconnect() callbacks for ARM CCA IDE setup Aneesh Kumar K.V (Arm)
2025-10-27 9:55 ` [PATCH RESEND v2 01/12] KVM: arm64: RMI: Export kvm_has_da_feature Aneesh Kumar K.V (Arm)
2025-10-29 16:39 ` Jonathan Cameron
2025-10-27 9:55 ` [PATCH RESEND v2 02/12] firmware: smccc: coco: Manage arm-smccc platform device and CCA auxiliary drivers Aneesh Kumar K.V (Arm)
2025-10-29 16:52 ` Jonathan Cameron
2026-01-06 12:33 ` Aneesh Kumar K.V
2025-10-27 9:55 ` [PATCH RESEND v2 03/12] coco: guest: arm64: Drop dummy RSI platform device stub Aneesh Kumar K.V (Arm)
2025-10-29 16:54 ` Jonathan Cameron
2025-10-27 9:55 ` [PATCH RESEND v2 04/12] coco: host: arm64: Add host TSM callback and IDE stream allocation support Aneesh Kumar K.V (Arm)
2025-10-29 17:18 ` Jonathan Cameron
2025-10-27 9:55 ` [PATCH RESEND v2 05/12] coco: host: arm64: Build and register RMM pdev descriptors Aneesh Kumar K.V (Arm)
2025-10-29 17:37 ` Jonathan Cameron
2025-10-30 8:44 ` Aneesh Kumar K.V
2025-10-30 10:00 ` Jonathan Cameron
2025-10-27 9:55 ` [PATCH RESEND v2 06/12] coco: host: arm64: Add RMM device communication helpers Aneesh Kumar K.V (Arm)
2025-10-29 18:33 ` Jonathan Cameron
2025-10-30 9:18 ` Aneesh Kumar K.V
2025-10-30 10:00 ` Jonathan Cameron
2025-10-30 14:04 ` Aneesh Kumar K.V
2025-10-30 18:02 ` Jonathan Cameron
2025-10-30 16:20 ` Aneesh Kumar K.V
2025-10-30 18:12 ` Jonathan Cameron
2025-10-31 8:04 ` Aneesh Kumar K.V [this message]
2025-10-31 12:07 ` Jonathan Cameron
2025-10-27 9:55 ` [PATCH RESEND v2 07/12] coco: host: arm64: Add helper to stop and tear down an RMM pdev Aneesh Kumar K.V (Arm)
2025-10-29 18:34 ` Jonathan Cameron
2025-10-27 9:55 ` [PATCH RESEND v2 08/12] coco: host: arm64: Instantiate RMM pdev during device connect Aneesh Kumar K.V (Arm)
2025-10-29 18:38 ` Jonathan Cameron
2025-10-27 9:55 ` [PATCH RESEND v2 09/12] X.509: Make certificate parser public Aneesh Kumar K.V (Arm)
2025-10-27 9:56 ` [PATCH RESEND v2 10/12] X.509: Parse Subject Alternative Name in certificates Aneesh Kumar K.V (Arm)
2025-10-27 9:56 ` [PATCH RESEND v2 11/12] X.509: Move certificate length retrieval into new helper Aneesh Kumar K.V (Arm)
2025-10-27 9:56 ` [PATCH RESEND v2 12/12] coco: host: arm64: Register device public key with RMM Aneesh Kumar K.V (Arm)
2025-10-29 17:19 ` Jason Gunthorpe
2025-10-29 18:53 ` Jonathan Cameron
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=yq5ams57cx9q.fsf@kernel.org \
--to=aneesh.kumar@kernel.org \
--cc=Suzuki.Poulose@arm.com \
--cc=aik@amd.com \
--cc=catalin.marinas@arm.com \
--cc=dan.j.williams@intel.com \
--cc=helgaas@kernel.org \
--cc=jgg@ziepe.ca \
--cc=jonathan.cameron@huawei.com \
--cc=kvmarm@lists.linux.dev \
--cc=linux-coco@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=sameo@rivosinc.com \
--cc=steven.price@arm.com \
--cc=will@kernel.org \
--cc=yilun.xu@linux.intel.com \
/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.