Re: [PATCH net-next 2/2] chcr: Add support for Inline IPSec
Hi Atul, Thank you for the patch! Yet we hit a small issue. [auto build test ERROR on net-next/master] url: https://github.com/0day-ci/linux/commits/Atul-Gupta/cxgb4-Add-support-for-Inline-IPSec-Tx/20171029-060344 config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 All errors (new ones prefixed by >>): In file included from drivers/crypto/chelsio/chcr_core.h:41:0, from drivers/crypto/chelsio/chcr_ipsec.c:64: drivers/crypto/chelsio/chcr_ipsec.c: In function 'chcr_ipsec_xmit': >> drivers/net/ethernet/chelsio/cxgb4/cxgb4.h:1678:13: error: inlining failed >> in call to always_inline 'cxgb4_reclaim_completed_tx': function body not >> available inline void cxgb4_reclaim_completed_tx(struct adapter *adap, ^~ drivers/crypto/chelsio/chcr_ipsec.c:597:2: note: called from here cxgb4_reclaim_completed_tx(adap, >q, true); ^ In file included from drivers/crypto/chelsio/chcr_core.h:41:0, from drivers/crypto/chelsio/chcr_ipsec.c:64: >> drivers/net/ethernet/chelsio/cxgb4/cxgb4.h:1678:13: error: inlining failed >> in call to always_inline 'cxgb4_reclaim_completed_tx': function body not >> available inline void cxgb4_reclaim_completed_tx(struct adapter *adap, ^~ drivers/crypto/chelsio/chcr_ipsec.c:597:2: note: called from here cxgb4_reclaim_completed_tx(adap, >q, true); ^ In file included from drivers/crypto/chelsio/chcr_core.h:41:0, from drivers/crypto/chelsio/chcr_ipsec.c:64: >> drivers/net/ethernet/chelsio/cxgb4/cxgb4.h:1687:13: error: inlining failed >> in call to always_inline 'cxgb4_ring_tx_db': function body not available inline void cxgb4_ring_tx_db(struct adapter *adap, struct sge_txq *q, int n); ^~~~ drivers/crypto/chelsio/chcr_ipsec.c:657:2: note: called from here cxgb4_ring_tx_db(adap, >q, ndesc); ^~~~ In file included from drivers/crypto/chelsio/chcr_core.h:41:0, from drivers/crypto/chelsio/chcr_ipsec.c:64: >> drivers/net/ethernet/chelsio/cxgb4/cxgb4.h:1687:13: error: inlining failed >> in call to always_inline 'cxgb4_ring_tx_db': function body not available inline void cxgb4_ring_tx_db(struct adapter *adap, struct sge_txq *q, int n); ^~~~ drivers/crypto/chelsio/chcr_ipsec.c:657:2: note: called from here cxgb4_ring_tx_db(adap, >q, ndesc); ^~~~ vim +/cxgb4_reclaim_completed_tx +1678 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h f2b7e78db drivers/net/ethernet/chelsio/cxgb4/cxgb4.h Vipul Pandya 2012-12-10 1562 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1563 void t4_wol_magic_enable(struct adapter *adap, unsigned int port, 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1564 const u8 *addr); 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1565 int t4_wol_pat_enable(struct adapter *adap, unsigned int port, unsigned int map, 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1566u64 mask0, u64 mask1, unsigned int crc, bool enable); 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1567 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1568 int t4_fw_hello(struct adapter *adap, unsigned int mbox, unsigned int evt_mbox, 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1569 enum dev_master master, enum dev_state *state); 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1570 int t4_fw_bye(struct adapter *adap, unsigned int mbox); 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1571 int t4_early_init(struct adapter *adap, unsigned int mbox); 625ba2c2e drivers/net/cxgb4/cxgb4.h Dimitris Michailidis 2010-04-01 1572 int t4_fw_reset(struct adapter *adap, unsigned int mbox, int reset); 636f9d371 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h Vipul Pandya 2012-09-26 1573 int t4_fixup_host_params(struct adapter *adap, unsigned int page_size, 636f9d371 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h Vipul Pandya 2012-09-26 1574unsigned int cache_line_size); 636f9d371 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h Vipul Pandya
Re: [PATCH net-next 1/2] cxgb4: Add support for Inline IPSec Tx
Hi Atul,
Thank you for the patch! Yet we hit a small issue.
[auto build test ERROR on net-next/master]
url:
https://github.com/0day-ci/linux/commits/Atul-Gupta/cxgb4-Add-support-for-Inline-IPSec-Tx/20171029-060344
config: x86_64-kexec (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/net//ethernet/chelsio/cxgb4/sge.c: In function 't4_eth_xmit':
>> drivers/net//ethernet/chelsio/cxgb4/sge.c:1198:6: error: implicit
>> declaration of function 'xfrm_offload'
>> [-Werror=implicit-function-declaration]
if (xfrm_offload(skb) && !ssi->gso_size)
^~~~
cc1: some warnings being treated as errors
vim +/xfrm_offload +1198 drivers/net//ethernet/chelsio/cxgb4/sge.c
1177
1178 /*
1179 * The chip min packet length is 10 octets but play safe and
reject
1180 * anything shorter than an Ethernet header.
1181 */
1182 if (unlikely(skb->len < ETH_HLEN)) {
1183 out_free: dev_kfree_skb_any(skb);
1184 return NETDEV_TX_OK;
1185 }
1186
1187 /* Discard the packet if the length is greater than mtu */
1188 max_pkt_len = ETH_HLEN + dev->mtu;
1189 if (skb_vlan_tagged(skb))
1190 max_pkt_len += VLAN_HLEN;
1191 if (!skb_shinfo(skb)->gso_size && (unlikely(skb->len >
max_pkt_len)))
1192 goto out_free;
1193
1194 pi = netdev_priv(dev);
1195 adap = pi->adapter;
1196 ssi = skb_shinfo(skb);
1197
> 1198 if (xfrm_offload(skb) && !ssi->gso_size)
1199 return adap->uld[CXGB4_ULD_CRYPTO].tx_handler(skb, dev);
1200
1201 qidx = skb_get_queue_mapping(skb);
1202 if (ptp_enabled) {
1203 spin_lock(>ptp_lock);
1204 if (!(adap->ptp_tx_skb)) {
1205 skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
1206 adap->ptp_tx_skb = skb_get(skb);
1207 } else {
1208 spin_unlock(>ptp_lock);
1209 goto out_free;
1210 }
1211 q = >sge.ptptxq;
1212 } else {
1213 q = >sge.ethtxq[qidx + pi->first_qset];
1214 }
1215 skb_tx_timestamp(skb);
1216
1217 cxgb4_reclaim_completed_tx(adap, >q, true);
1218 cntrl = TXPKT_L4CSUM_DIS_F | TXPKT_IPCSUM_DIS_F;
1219
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [Part2 PATCH v6 13/38] crypto: ccp: Add Secure Encrypted Virtualization (SEV) command support
On 10/27/17 7:00 PM, Borislav Petkov wrote:
> On Fri, Oct 27, 2017 at 05:59:23PM -0500, Brijesh Singh wrote:
>> Yes it is typo. PEK_GEN wants FW to be in INIT state hence someone need
>> to transition from UNINIT -> INIT.
> Which, once you've done it once on driver init, is there.
>
>> That's what I am doing except FACTORY_RESET.
> Well, not really. Lemme pick a command at random...
>
> PEK_CSR. For that, you do INIT -> PEK_CSR -> SHUTDOWN.
>
> Doc says, platform needs to be in INIT or WORKING state. But nothing
> says you should shut it down. Spec says, SHUTDOWN transitions platform
> to UNINIT state. So when the next command comes in which needs the
> platform to be in INIT state, you go and INIT it again. For no reason
> *WHATSOEVER*!
>
> I know, you're gonna say, but what if the next command needs a different
> state than INIT. Well, *then* you transition it, in the command
> function. When that function executes. But not before that and not in
> preparation that *maybe* the next command will be it.
>
> Now, if you did:
>
> INIT once during driver init
>
> PEK_CSR
>
> (platform remains in INIT state)
>
> <--- the next command here can execute directly if it is allowed in INIT
> state.
>
> Instead, the platform has been shutdown and you init it again. Do you
> see now what I mean?
Yes, I can see that with your proposal we may able to save some PSP
interaction because after command execution we do not restore to the
previous state. e.g before executing the PEK_CSR command if FW was in
UINIT then we do UNINIT -> INIT and leave it to INIT state.
> IOW, once you init the PSP master, you should keep it in the INIT state
> - or the state in which most commands expect it to be and thus save
> yourself all that unnecessary toggling. If a command needs it to be in a
> different state, only *then* you transition it.
Let me implement it and send you the patch. I think the command
execution function will look like this:
static int sev_ioctl_do_pek_csr(...)
{
mutex(_init_mutex);
/* If FW is not in INIT state then initialize before executing command */
if (psp->sev_state != SEV_STATE_INIT) {
rc = sev_platform_init(...);
if (rc) {
mutex_unlock(_init_mutex);
return rc;
}
}
rc = sev_do_cmd()
mutex_unlock(_init_mutex);
return rc;
}
and factory reset will look like this
static int sev_ioctl_do_reset(...)
{
mutex(_init_mutex);
/* If FW is not in UINIT state then shutdown before executing command */
if (psp->sev_state != SEV_STATE_INIT)
sev_platform_shutdown(...);
rc = sev_do_cmd()
mutex_unlock(_init_mutex);
return rc;
}
> Instead, what you have now is that you call INIT and SHUTDOWN
> around SEV_PEK_GEN, SEV_PDH_GEN, SEV_PEK_CSR, SEV_PEK_CERT_IMPORT,
> SEV_PDH_CERT_EXPORT and for all those, the platform must be in INIT
> (for some in WORKING state) but for all in INIT state and "The platform
> remains be in the same state after completion." So the whole SHUTDOWN ->
> INIT wankery in-between is a pure waste of electrons.
>
>> I see that we can do a small optimization -- since we already know
>> the FW state hence we can avoid issuing PSP command when we know for
>> sure that command will fail because we are not in correct state.
> As I said before, you should do that regardless by recording the current
> state of the PSP in variable so that you can save yourself the status
> querying.
>
>> If command needs INIT state and FW is not in INIT state then its safe to
>> transition from UNINIT -> INIT. But if command needs UNINIT state and FW
>> is in INIT state then its not safe to transition -- in those case we
>> simply return EBUSY and let the user retry the command.
> Whatever - that doesn't contradict what I'm proposing.
>
