RE: [PATCH] Bluetooth: btrtl: Avoid loading the config file on security chips

[Max Chou <max.chou@realtek.com>]

Hi! Paul,

Thanks for your response.
Please check my comments inline.


> -----Original Message-----
> From: Paul Menzel [mailto:pmenzel@molgen.mpg.de]
> Sent: Wednesday, November 5, 2025 3:58 PM
> To: Max Chou <max.chou@realtek.com>
> Cc: Marcel Holtmann <marcel@holtmann.org>; Luiz Augusto von Dentz
> <luiz.dentz@gmail.com>; linux-bluetooth@vger.kernel.org; linux-
> kernel@vger.kernel.org; Hilda Wu <hildawu@realtek.com>;
> alex_lu@realsil.com.cn; niall_ni@realsil.com.cn; KidmanLee
> <kidman@realtek.com>
> Subject: Re: [PATCH] Bluetooth: btrtl: Avoid loading the config file on security
> chips
> 
> 
> External mail : This email originated from outside the organization. Do not reply,
> click links, or open attachments unless you recognize the sender and know the
> content is safe.
> 
> 
> 
> Dear Max,
> 
> 
> Thank you for your patch.
> 
> Am 05.11.25 um 07:37 schrieb Max Chou:
> > For chips with security enabled, it's only possible to load firmware
> > with a valid signature pattern.
> 
> How can security be enabled?
> 
> What is currently logged? An error?


The security chips will be shipped to specific brand customers and are managed by an eFuse, which is programmed during manufacturing.
Currently, loading the config file causes initialization to fail on security chips.
The security chips can only load firmware files that contain a valid signature.


> 
> Please go into the changes. What is the vendor command 0xAD over 0x0D?
> 
Actually, the current value is incorrect. The correct value for the parameters of the vendor command is 0xAD.

> > - Example log for a security chip.
> >
> > Bluetooth: hci0: RTL: examining hci_ver=0c hci_rev=000a
> >    lmp_ver=0c lmp_subver=8922
> > Bluetooth: hci0: RTL: rom_version status=0 version=1
> > Bluetooth: hci0: RTL: loading rtl_bt/rtl8922au_fw.bin
> > Bluetooth: hci0: RTL: cfg_sz 0, total sz 71301
> > Bluetooth: hci0: RTL: fw version 0x41c0c905
> >
> > - Example log for a normal chip.
> >
> > Bluetooth: hci0: RTL: examining hci_ver=0c hci_rev=000a
> >    lmp_ver=0c lmp_subver=8922
> > Bluetooth: hci0: RTL: rom_version status=0 version=1
> > Bluetooth: hci0: RTL: loading rtl_bt/rtl8922au_fw.bin
> > Bluetooth: hci0: RTL: loading rtl_bt/rtl8922au_config.bin
> > Bluetooth: hci0: RTL: cfg_sz 6, total sz 71307
> > Bluetooth: hci0: RTL: fw version 0x41c0c905
> >
> > Tested-by: Hilda Wu <hildawu@realtek.com>
> > Signed-off-by: Nial Ni <niall_ni@realsil.com.cn>
> > Signed-off-by: Max Chou <max.chou@realtek.com>
> > ---
> >   drivers/bluetooth/btrtl.c | 24 +++++++++++++-----------
> >   1 file changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> > index 8290932b8f7b..f6fccc6fdf22 100644
> > --- a/drivers/bluetooth/btrtl.c
> > +++ b/drivers/bluetooth/btrtl.c
> > @@ -50,7 +50,7 @@
> >
> >   #define     RTL_CHIP_SUBVER (&(struct rtl_vendor_cmd) {{0x10, 0x38, 0x04,
> 0x28, 0x80}})
> >   #define     RTL_CHIP_REV    (&(struct rtl_vendor_cmd) {{0x10, 0x3A, 0x04,
> 0x28, 0x80}})
> > -#define      RTL_SEC_PROJ    (&(struct rtl_vendor_cmd) {{0x10, 0xA4, 0x0D,
> 0x00, 0xb0}})
> > +#define      RTL_SEC_PROJ    (&(struct rtl_vendor_cmd) {{0x10, 0xA4, 0xAD,
> 0x00, 0xb0}})
> >
> >   #define RTL_PATCH_SNIPPETS          0x01
> >   #define RTL_PATCH_DUMMY_HEADER              0x02
> > @@ -544,7 +544,6 @@ static int rtlbt_parse_firmware_v2(struct hci_dev
> *hdev,
> >   {
> >       struct rtl_epatch_header_v2 *hdr;
> >       int rc;
> > -     u8 reg_val[2];
> >       u8 key_id;
> >       u32 num_sections;
> >       struct rtl_section *section;
> > @@ -559,14 +558,7 @@ static int rtlbt_parse_firmware_v2(struct hci_dev
> *hdev,
> >               .len  = btrtl_dev->fw_len - 7, /* Cut the tail */
> >       };
> >
> > -     rc = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
> > -     if (rc < 0)
> > -             return -EIO;
> > -     key_id = reg_val[0];
> > -
> > -     rtl_dev_dbg(hdev, "%s: key id %u", __func__, key_id);
> > -
> > -     btrtl_dev->key_id = key_id;
> > +     key_id = btrtl_dev->key_id;
> >
> >       hdr = rtl_iov_pull_data(&iov, sizeof(*hdr));
> >       if (!hdr)
> > @@ -1081,6 +1073,8 @@ struct btrtl_device_info *btrtl_initialize(struct
> hci_dev *hdev,
> >       u16 hci_rev, lmp_subver;
> >       u8 hci_ver, lmp_ver, chip_type = 0;
> >       int ret;
> > +     int rc;
> > +     u8 key_id;
> >       u8 reg_val[2];
> >
> >       btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL); @@ -1191,6
> > +1185,14 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
> >               goto err_free;
> >       }
> >
> > +     rc = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
> > +     if (rc < 0)
> > +             goto err_free;
> > +
> > +     key_id = reg_val[0];
> > +     btrtl_dev->key_id = key_id;
> > +     rtl_dev_dbg(hdev, "%s: key id %u", __func__, key_id);
> > +
> >       btrtl_dev->fw_len = -EIO;
> >       if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev == 0x000c) {
> >               snprintf(fw_name, sizeof(fw_name), "%s_v2.bin", @@
> > -1213,7 +1215,7 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev
> *hdev,
> >               goto err_free;
> >       }
> >
> > -     if (btrtl_dev->ic_info->cfg_name) {
> > +     if (btrtl_dev->ic_info->cfg_name && !btrtl_dev->key_id) {
> 
> So on non-security enabled chips, key_id is 0? It’d be great if that could be made
> clear in the commit message.


As you understood, key_id is 0 for normal chips.
I will add this information to the commit log in the v2 patch.


> 
> >               if (postfix) {
> >                       snprintf(cfg_name, sizeof(cfg_name), "%s-%s.bin",
> >                                btrtl_dev->ic_info->cfg_name, postfix);
> 
> 
> Kind regards,
> 
> Paul


BRs,
Max